Coding Guideline : Methods shouldn't contain more than 7 statements?

Question

I was looking through the AvSol Coding Guidelines for C# and I agree with nearly everything but I'm really curious to see what other think of one specific rule.

AV1500

Methods should not exceed 7 statements A method that requires more than 7 statements is doing too much, or has too many responsibilities. It also requires the human mind to analyze the exact statements to understand what the code is doing. Break it down in multiple small and focused methods with self-explaining names.

Do most of you follow this rule? Even if there's little to be saved from creating a new method (Your code is still DRY) aside from greatly increasing readability? And is your number still as low as 7? I would tend more toward 10.

I'm not saying I violate this rule all over the place--on the contrary, my methods are 95% small and focused but saying you should never violate this rule really blew me away.

I really just want to know what everyone thinks of NEVER violating this rule (It's a '1' on the coding standard--meaning NEVER do this). But I think you'd have trouble finding a codebase that doesn't.

Answer 1

This is a "standards smell" to me. Whenever I see coding standards with specific limits in them, I worry. You almost always run into a case where a method needs to be bigger than the standard allows (whether it's line length/count, number of variables, number of exit points, etc). Standards should be more like guidelines, and allow sufficient leeway for exercising good judgement. Don't get me wrong, it's good to have standards, but they shouldn't become "micromanagement by proxy".

Answer 2

It is usually a good idea to split stuff into little methods. But the important thing is to split things where it make sense.

If it doesn't make sense to split, then don't split. This is often the case for some procedures or GUI code.

Steve McConnell stated in Code Complete that you aren't always more productive when using short methods. If you split when it doesn't make sense, you add complexity to the code for no benefit.

As always with guidelines, it is good to remember why the constraints exist, so you can learn when it doesn't apply. In most of the code the methods will be short, or you probably have a problem with DRY or separation of concerns. But if isn't the case, then fine.

Answer 3

It should be regarded as a rule of thumb.

Things like "No more than 80(100,120) columns of text", "one exit point per method", "no more than 2 levels of nesting", are what I would call thresholds for indicators of code smell. If you violate them on occasion, that does not necessarily mean the code is bad. If you find yourself violating them consistently then something smells in the code and you may wish to take a pause and rethink your approach.

To me, the most important criteria are, "Is this code understandable?", "Is it repetitive?", "Is it broken up in logical places?", "Is it loosely coupled?" There are a few more, but I think the basic idea can be summed up by remembering Donald Knuth's advice: "Programs are meant to be read by humans and only incidentally for computers to execute."

Answer 4

I've never taken the time to actually count the number of statements in my methods, but I do strive to write methods that cleanly perform a single, clear purpose. As long as your code is clean, readable, and follows the DRY and Single Responsibility principles, you've probably done your job. I think that arbitrarily splitting a method apart just to enforce the seven-statement limit could make your code less readable/maintainable.

Answer 5

It's approximate

These kinds of rules shouldn't be taken too literally. They could have said "methods should be short". However, some people would have interpreted that as "less than 1 page" and others as "2 lines at most."

I would assume that they said "7 statements" to give you a rough idea (though I think they should have said "about 7"). If you need 9 once in a while, don't sweat it. But if you're hitting 20, you'll know you're not in the right ballpark for this rule.

Answer 6

7 is a completely arbitrary number with absolutely no significance.

Cyclomatic complexity is a bigger issue than the number of statements. I've seen code that had 100s of statements in a single method (which I thought was terrible), but it had a cyclomatic complexity of 1 and it really only did 1 thing. There were just a lot of steps. We discussed breaking it apart into smaller methods, but those methods would only be called by this one method.

While that is a fairly extreme case, the point is you need to keep the code DRY and a low cyclomatic complexity. That is more important than the number of lines in a method.

Take a switch / case statement for example. If you have more than 7 possible values do you need to break the evaluation into multiple methods? Of course not, that would be silly.

Artificially breaking code into more methods just to keep the number of statements under 7 only makes your code worse.

The guideline should be Each method should do 1 thing and keep your code DRY.

Answer 7

Well, it depends a bit. I write a lot of code that accesses databases. The boiler plate code for exception handling is more that seven statements long in a lot of cases. I'd say teh best guideline is to make sure your function has one purpose

Answer 8

Everything is a trade-off. The problem with the proposed approach - refactoring into several methods and classes so that each method is short - is that although for different reasons, it leads to unreadable code when taken to its extreme.

Imagine a method foo() that does 7 things. You might argue that 7 things is too much. Maybe in many cases you're right. On the other hand, these 7 things might be closely related; the logic may flow smoothly and read like prose; you may have no trouble understanding it when you actually need to. What might end up much worse is to have those 7 things distributed across a large source tree, so that if you look at foo() you have no idea what it does without looking in 7 different places.

Many people get rules like this in their head, and the result is what I think of as OO spaghetti. Everything is neat, boxed in its own little method or class, with atomic little micro-transactions occurring in each place. But it's impossible to come fresh to such a code base and know what it's doing. You become lost.

Answer 9

it's not a bad guideline. I've never regretted splitting up methods and classes (I've never found I had too many) as long as they are grouped and interrelated well enough.

The trick is to NOT split it vertically (Just pinch a method off at one point and start a new one). The trick, like with unit testing, is to keep such a rule in mind from the start so that you actually design better, passing off 3 or 4 statements mid-method to another method because having a method call describes what you are doing better than those 3 or 4 statements in the middle of your code.

This kind of split, even if it's arbitrary and only used once, may lead to better refactorings later due to a new clarity of code, this is true of smaller classes as well.

Think of it like you would unit testing. If you try to add unit testing after the fact it's difficult and sometimes seems impossible but if you design it in from the beginning it actually makes all your code better.

Summary? If comparing the design smell of "Use less than 7 statements" to the code smell of "I used more than 7 statements", I'd rather eliminate the code smell.

Answer 10

Wow! I never expected to find such an intense discussion on a simple guideline that roughly says that your methods should be very small. Because they're always developers who want their guidelines to be explicit, I choose 7 because that sounded like a nice threshold.

Some of you have already quoted the disclaimer at the beginning of the document. But just to be clear, this document represents a collection of guidelines which try to help you write better code and design better systems. I've never stated that anything should be a rule, even though a guideline is marked as a level 1. Those levels are simply the collective opinion of the many people who have been using this document for a while.

I have also never claimed to be an expert. But I've been in this profession for 15 years now, with about 4 years of C++ experience and 11 years of C# experience. It was originally based on Industrial Strength C++, but I have been refining it since then with input from the community.

Regardless, the point I was trying to raise is that you should keep thinking for yourself. If you think the 7 statements guideline is not useful, than just make it longer. Heck, I even violate that guideline once in a while. I just violate it consiously and accept the consequences.

Answer 11

Firstly: it's a guideline, not a rule. It's called a guideline, so please treat it as such. This implies that your own judgement is also required (as always)

That apart, I can think of plenty examples of good code that doesn't adhere to this constraint. Even though it's just a guideline, it's a poor one.

Answer 12

I agree with the statement above me. This is just guideline and in a perfect world everything would be objects, reused in every program, and the world would be a beautiful place. It is not always true and sometimes that would lead to a lot of overhead or wasted resources. You need to put that in mind also.

Answer 13

Pretty silly when you add exception handling into the mixture.

After "try, catch , finally" you are left with four statements per method!

Also consider a method "validateForm" for a 20 field form, even if you handle all the individual validations in separate methods you still have 20 field validation methods to invoke. According to these guidlines you would end up with some pointless split like "validateTopOfScreen", "validateMiddleOfScreen" and "validateBottomOfScreen".

Answer 14

The question is conflating "rules" with "guidelines". Rules are meant to be obeyed - guidelines are advice that are meant to cause you to consider what you are doing and if it really could be done in a better way.

I'd say, on average, most programming is probably improved by following guidelines but there will always be cases where following the guidelines dogmatically will cause more problems than they were intended to resolve. That's why they are not presented as rules, but guidelines.

A previous answerer has shown that the writers of the guidelines never intended them to be applied dogmatically and included statements to that effect in their document.

Answer 15

I agree with soe of the comments above. 7 for me is arbitary and may be useful in some languages where ruby, but for languages like C#, Java, C++ I doubt this "7 line" convention. Let me give you an example.My current application has 22 text fields and I do server side validation. The method is called validateInput() and my preference is validate all the fields in one method itself unless I have validate something complex like checkEmailFormat(). So basically my validateInput method code is 108 lines with occasional calls to complex validation.

Now imagine if I called 25 methods to validate each field. If a new developer comes in he will have to go in and out of the main method to jump through 25 methods which in turn might be calling a few other. He is bound to get lost in big projects.

What I really do to make my code clear is provide clean comments which basically say what these 4 lines are doing ex -

validateInput(UserObj user){

//Validate first name ....... ......

//validate last Name ...... ......

//validate email ..... //too complex to check email format regular expresion checkEmailFormat(); .... .....

and so on.. }

Answer 16

Bad rule. With code for exception handling, setting database fields, validation, how can most methods be under seven statements? Should we never do inline lambda functions?

Lines of code are an arbitrary count of complexity. With extension methods I can create code like

                Parallel.ForEach(regions, region => {   Console.Write(region.Resorts.Where(x => x.name == "Four Seasons").OrderBy(x => x.RegionId).ThenBy(x => x.ResortId).ThenBy(x => x.name).ToList());   });

Answer 17

All junior programmers should be forced to adhere to this principle. That would force them to think about the structure of what they're busy with, instead of just adding more and more lines code.

I'm currently looking at a method of 550 lines of code with lots of if else statements and continues and returns weaved into it. It's crap. Had the programmer only been forced to think about what he was doing...