Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExcessiveMethodLength counts "{" and "}" as part of the function length #54

Open
insha opened this issue Apr 28, 2017 · 1 comment
Open
Assignees

Comments

@insha
Copy link

insha commented Apr 28, 2017

If I have a method like:

func init
{
  let something = 1
  let another = somethingElse(start: 0,
                                end: 1)
  let anArray = [
    "1",
    "2",
    "3"
  ]
  
  if something == 1
  {
    dosomething()
  }
}

What should be the length for this function? 5 or 14?

My code style is that the { and } are on a line by itself for all keywords that require it, so this is causing false positives in my code. Functions that are really under 20 lines are being flagged due to brace being on a line by itself and some function calls that have their arguments stacked (as in the above).

Also when an array literal is defined, see above, all of the lines are counted as part of the function. While the lines are part of the function, they are not individual lines. The declaration of an array is a single line that is split into multiple lines.

Note: I would love to help out with this project. How can I help?

@thelvis4 thelvis4 assigned S2dentik and unassigned S2dentik Apr 28, 2017
@S2dentik
Copy link
Contributor

S2dentik commented Apr 29, 2017

Hey @insha, thanks alot for reaching! So I'll try to answer your questions one by one:

  1. What should be the length for this function?
  • Length of this function should be twelve. This is calculated based on some community guidelines, one of which being Methods should not have more than an average of X code lines (not counting line spaces and comments)., in our case also including opening and closing brace. These follow a simple purpose: easy to see and understand. One drawback is as we see in your case, a line with a single brace is also counted, but that is intended as it plays it's own role in code readability.
  1. So is it a bug in the end?
  • Yes. In the end there is a bug here, but in the following:
func test() { 
    let a = 1
}
func test2()
{
    let a = 2
}
func test3()

{
    let a = 3
}

Even if bodies of these 3 functions are equal (have 3 lines), they are reported differently by Taylor. We should also rethink if we count the { and } in the body.

  1. How can I help?
  • We are very open to PRs and issues and we'll try to work them out with you until we they can be merged. By contributing you'll learn a whole lot about code quality, which will sure make you reason differently about code. Taylor can be contributed to either by refactoring some of the existing parts or by adding some rules. Plus one of our main concerns was performance, so if anything that improves it is welcomed. If you want to go into details on this, just reply in this thread on where you'd like to help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants