Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Most people seem to agree that long methods are a code smell - a sign something may not be quite right with the code contained in them.

Which tools do you use to detect this smell?

clarified title based on responses. also, remember:

  • Your code will live over time, and be edited by multiple programmers
  • Emergency fixes and changes will come in, late at night, when the writer is too tired to pay attention to smells
  • Different programmers use different tools. A contractor with 4 screens set at maximum resolution will have a different idea of acceptable method size

In this context, I'm looking for tools and methods which go beyond looking at the size of a method when it's written, or when it's being edited.

share|improve this question

closed as not constructive by Anna Lear Dec 16 '11 at 3:48

As it currently stands, this question is not a good fit for our Q&A format. We expect answers to be supported by facts, references, or expertise, but this question will likely solicit debate, arguments, polling, or extended discussion. If you feel that this question can be improved and possibly reopened, visit the help center for guidance.If this question can be reworded to fit the rules in the help center, please edit the question.

7  
Who needs a tool? If it does more than one thing, it's too long. You should be able to understand the purpose, and workings of a method in a few moments –  CaffGeek Feb 3 '11 at 21:30
    
Well, sure, that works when you write the method. What about 10 years, 40 developers, and 250kloc later? –  blueberryfields Feb 3 '11 at 21:43
1  
@blueberryfields: At that point, don't bother. Unless you have a budget for a refactoring project (or enough time/money in some other project) leave the code alone if it works. –  FrustratedWithFormsDesigner Feb 3 '11 at 21:45
3  
@blueberryfields, if a single method is 250k lines...flee. –  CaffGeek Feb 3 '11 at 22:05
1  
@blueberryfields, that's because you can't maintain a codebase that size as part of your daily work. You maintain what you touch, as needed. As to the rest, if it ain't broke, don't fix it. –  CaffGeek Feb 3 '11 at 22:18

9 Answers 9

up vote 7 down vote accepted

My thumb, and my eye. If it's annoying to maintain, or just looks bad, I refactor. Nothing fancy.

share|improve this answer

I agree with the other answers in that usually there is no need for any fancy tool. I used to follow the "fits to the monitor at once" rule (with the extension that "using a normal size font, i.e. no smaller than 8pt", as one of my ex-coworkers used 6pt). That was in my practice about 30-50 lines max.

Then I read Clean Code and that pushed my threshold down to about 10 lines. But the main point is not really the lines of code, rather

  • cohesion (each method doing one thing), and
  • staying on the same level of abstraction (e.g. not calling high level API methods and doing bit flipping within the same method).

So most of my methods nowadays are at most 5 lines long, but occasionally some may be 25 when I can't see an easy way to refactor.

However, if you really need an objective metric, check out cyclomatic complexity. Some IDEs like IntelliJ have it built in their static code analysis tool.

If you want tool support to identify long methods or other code smells within a large legacy codebase, this metric may help you. But I usually prefer to focus on the areas I need to touch anyway. Since the reason to touch the code is a) there is a bug in there to fix, b) there is a feature to add, these tend to be the most critical code parts over the long term. There are typically lots of ugly code in other places of the system too, but as long as there are no known bugs in there and no business request to change anything there, there is little value in refactoring those parts.

share|improve this answer

Your head.

From Practical Common Lisp:

"How do you know when a function or method is too big? Well, said the candidate, I don't like any method to be bigger than my head. You mean you can't keep all the details in your head? No, I mean I put my head up against my monitor, and the code shouldn't be bigger than my head."

share|improve this answer

You have to scroll to see whole method

share|improve this answer
    
That works for looking at methods one at a time - good when writing the method, or doing a detailed code review. How would I know how my whole code base is doing, though, when it comes to this smell? –  blueberryfields Feb 3 '11 at 21:32
1  
So if I get one of those monitors that rotates to portrait format, or just set a very very high resolution, does that make the code better (since there's no scrolling)? ;) –  FrustratedWithFormsDesigner Feb 3 '11 at 21:38
    
@FrustratedWithFormsDesigner: And that's why I answered the way I did. –  Jon Purdy Feb 3 '11 at 21:40
2  
Adding to @Frustrated, what if one dev had a tiny 1024x768 CRT monitor while another has a giant 2560×1600 widescreen LCD monitor? –  TheLQ Feb 3 '11 at 22:02
    
@Frustrated: Of course it's a rule of thumb, so you have to adjust this to your environment :) –  Simon Feb 9 '11 at 8:20

Gut feeling proves a very helpful tool.

share|improve this answer

I've seen tools like JTest, but I prefer to do a manual inspection. Things to look for: Repeated code that could be refactored; also very deeply nested code that might be refactorable. As I recall, JTest just used LOC to say if a method was too long, and I think that's a little too simplistic.


Actually, I think JTest also had a refactoring suggestion component that would look for repetitive code and suggest it be refactored. It was slow and didn't always work well (which might be why I stopped using it), but it (or something like it) might help in situations like this.

share|improve this answer
    
A manual inspection might work on a very small codebase.... How often do you manually go through all of your code, and how long does it take? –  blueberryfields Feb 3 '11 at 21:41
    
@blueberryfields: I will do this on my code that is in active development. I will do this on code other devs check in (I see it when I sync to source control) for the same project and make suggestions, or even make the changes myself (time permitting). Sometimes they may do the same to my code, and I'm cool with that. I do not do this on large, pre-existing codebases. There just isn't the time and even if I did find smelly code I doubt I'd get a project's worth of resources to fix those problems, especially if the system "just works". –  FrustratedWithFormsDesigner Feb 3 '11 at 21:47
    
The question of how you get resources to do proper maintenance on the code is probably out of scope here - I'm just looking to find out, how do you know which code needs maintenance in the first place. –  blueberryfields Feb 3 '11 at 21:59
    
@blueberryfields: I think JTest was the only tool that could help with finding code that needed maintenance in the first place. You'd run it on a large codebase (because a small codebase could probably be done by hand) and after a while (usually a couple hours) you'd get some results. I didn't always think it was doing a great job, but some things it was ok with. It can be endlessly tweaked and configured and maybe if the configuration had been better we'd have had better results with it? –  FrustratedWithFormsDesigner Feb 3 '11 at 22:02

There are some good code quality analysers around, such as Sonar.

One of its features is to measure the Cyclomatic Complexity of methods. Sonar works on the java platform but there are plug ins for;

  • Flex
  • PHP
  • PL/SQL
  • Cobol
  • Visual Basic 6.
share|improve this answer

My personal limits are around 7 lines in a method/function (easily seen visually), 300 lines in a Class (easily noticed by line number) and about 10 Classes in a package. I don't use any tools to detect these, and I don't force adherence to it. But when I reach any of these limits I strongly question whether a refactor is appropriate.

share|improve this answer
    
Those numbers sound rather arbitrary... Any reasons to them? –  FrustratedWithFormsDesigner Feb 3 '11 at 22:04
    
@FrustratedWithFormsDesigner They are just what has have evolved into my workflow over time. I guess when the code gets near these limits my gut reaction is that there is probably a code/design smell or likleyhood of duplication creeping in. –  Alb Feb 3 '11 at 22:09

I think this depends on the language and development tools (e.g. IDE) that you are using. Some development tools can do static code analysis for you and analyze your codebase for any issues. For instance, certain Visual Studio editions and accompanying tools can calculate specific code metrics and alert you to parts of your code that you should consider refactoring.

Overall, you have to use your best judgment and allow the code to be readable and maintainable. Code reviews wouldn't hurt if you work in a team of developers and you should encourage everyone in the team to either refactor their own code or bring examples of code that should be reviewed/refactored to team meetings. Human intervention in this case will likely produce better results than any tool can.

share|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.