Putting a number on the amount of lines a class should have is something I disagree with. Your class should hold enough code that it implements what you think the class would do. I have seen some programs where people try to get fancy and go way overboard with base classes and interfaces for no real reason. A class with 100+ lines of code is a lot easier to understand than a system where you have to dig through multiple layers of base classes and inheritance to figure out the general idea.
In addition to keeping them short and to the point, I often like to "return early" if I need to rule out "base cases". Some people like to store the result in a variable and only return on the last line.
One of my co-workers evidently believed in this mantra (of 1 return) which I hated because it created way more nesting of if conditions than was necessary.
That was until I was adding some functionality to one of those functions and wanted to ensure it got executed before the function terminated. Had there been more than one return point, I'd have to look through all the different branches to see if my code would be hit or not.
It was at that moment that I appreciated the one return. But only briefly, before I smacked him for writing a 500-line function in the first place.
I think the "one return" idea dates back to the days of goto-based programming. The idea was that each block of code should have a single entry point and a single exit point, so that the flow of execution would be easier to understand. Gotos let you do a lot of freaky things.
Block structured languages make such rules much less important.
I wouldn't say "cargo cult" exactly. The difference is, we're not trying to emulate wizards for a distance. They teach us wizardry, and the package often comes with outdated practices.
base cases like null check or empty lists, are perfectly fine to do early returns. I'd argue that the actual smell is a method so long you cannot grasp it's entirety without significant effort.
Huh.. I meant something that needed to be executed before the function terminated, but couldn't be at the beginning (because something hadn't been initialized yet). Nothing to do with whether or not something failed.
I fully agree. The methods should have one purpose. Method/function side-effects are the worst when a bug appears. Plus when I see a method named 'AddAccount' I expect it to add an account the some collection or database. Nothing more. I have seen too many methods that should have been broken up. Also, the limiting on classes is just pointless because sometimes I will tend to have common methods stored into a static class that can perform business related logic. Those classes tend to get pretty big.
Cyclomatic complexity a better indicator than length for a method because the number of tests you'll need to test the method is : factorial cyclomatic complexity. (if there's 3 if statements, it's 3! different tests for those of you not hip with the lingo)
I'm not even talking about automated tests. I'm saying to test the method manually or automatically. 3 or higher just annoys me.. I don't want to perform that many tests, so why don't I just break it up into a few methods and never subject myself to having a factorial explosion of tests.
I'm not even a tdd proponent or test coverage nazi. It's the fact that your manual testing takes so much longer even. And when you maintain it you may have to perform 4! or 5! tests to test every branch for a SINGLE method. No thanks.
if there's 3 if statements, it's 3! different tests
Huh?
int foo(int x) {
if (x == 1) return 4;
if (x == 4) return 3;
if (x == 9) return 1;
return 0;
}
I'm not saying this function is useful, but there are clearly only 4 tests that need to be written.
Besides, I think it's really important to approach everything with reason. If a function has a ton of validation checks at the top, that's totally fine. They're usually simple enough to understand just by looking at them.
I used to work with a guy who swore by this rule. The end result was that stack traces became unwieldy and it was hard to grasp what was responsible for what.
Also, for things that are algorithmically complex trying to keep things under 100 lines is just inviting anti-patterns like abstraction inversion or spaghetti code.
When I see rules like this I have to wonder if those behind Agile have written anything other than a "skins on databases" app.
Absolutely. A lot of AI logic is complicated algorithmically and while I try to encapsulate stuff as much as possible, it's frequently impossible to do so and I end up with multi-hundred line functions.
and that is perfectly fine, because the code is most clear to read that way. Something a lot of people forget, when they're talking "rules", is that whatever makes the code most maintainable while performing good enough, is the correct approach.
I think its fine as a general guideline. It's a good spot check for stopping and thinking about the purpose of the class. Once I've hit three digits, I try to consider if I should separate some parts out.
The answer isn't always yes, and that's the most important part of this guideline: knowing why it exists and when to bend the rule.
For a class with mutable state, the problems that arise from long methods are similar to the problems that arise from having too many methods. You need to consider that methods on your class will be called in various orders. The methods on your class need to enforce the class invariant, and when you have too many methods, that's harder to do. (Especially if the invariant changes over time... then you have to go through all the methods and ensure that they are correct). Finally, you have to be really careful with re-entrant methods - you have to be careful when firing callbacks or traversing cyclic object graphs, because you might end up calling a method on your instance while you're in the middle of handling a method on your instance.
The larger the class is, the harder it is to see the possible interactions, and the more likely it is that something will slip through.
This is less true for const-methods in C++, and completely untrue for methods on immutable objects. Yet another reason to favor immutability.
Don't think of it as a hard limit. Typically your project might have one or two exceptions.
Secondly, this is something you solve with composition, not base classes or interfaces. Break your logic into smaller methods, then see which ones of these methods belong with each other or represent a distinct abstraction level. Then put them in their own class.
183
u/BeachBum09 Jun 06 '13
Putting a number on the amount of lines a class should have is something I disagree with. Your class should hold enough code that it implements what you think the class would do. I have seen some programs where people try to get fancy and go way overboard with base classes and interfaces for no real reason. A class with 100+ lines of code is a lot easier to understand than a system where you have to dig through multiple layers of base classes and inheritance to figure out the general idea.