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.

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.

share|improve this question
42  
Do they count case statements in a singe switch too? Any way, it's nothing but an idiotic, useless requirement. Those who wrote it know nothing about programming. –  SK-logic Feb 27 '12 at 15:58
79  
The only rule that should be a '1' on the coding standard should be the rule "thou shalt take all coding guidelines with a grain of salt." –  Mike Nakis Feb 27 '12 at 16:00
6  
@SK-logic 1027 ain't bad either -- it must be fun writing code which has to handle missing data, if you have to treat empty string as equal to null string. –  quant_dev Feb 27 '12 at 16:34
18  
When reading this guideline I would expect to see a codebase full of: void DoSomething() { DoSomethingFirstSevenLines(); DoSomethingSecondSevenLines(); DoSomethingThirdSevenLines(); etc; } –  tugs Feb 27 '12 at 22:46
7  
Try reflecting most of the .NET Framework and see how many methods have fewer than 7 statements... –  David Boike Feb 28 '12 at 4:53
show 14 more comments

17 Answers 17

up vote 172 down vote accepted

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".

share|improve this answer
19  
+1 for "micromanagement by proxy." I suspect that someone read about the "7 plus or minus 2" rule (en.wikipedia.org/wiki/…) and confused short-term fact retention with long-term storage like, you know, a code editor. –  fluffy Feb 27 '12 at 16:41
19  
Any sensibly thought-out standard prohibits magic numbers in code, right? You'd think that the coding standard would be held to that standard, too. The number 7 definitely has a certainly "magic" quality to it. –  Adam Crossland Feb 27 '12 at 16:47
2  
Your answer would make more sense if the title of the document wasn't "Coding Guidlines for C# 3.0 and 4.0. –  Andy Feb 28 '12 at 0:45
3  
I downloaded this document and started looking through it. In 1.6, it states: "The document does not state that projects must comply with these guidelines, neither does it say which guidelines are more important than others. However, we encourage projects to decide themselves what guidelines are important, what deviations a project will use, who is the consultant in case doubts arise, and what kind of layout must be used for source code." Even though a 1 means "Guidelines that you should never skip and should be applicable to all solutions", you can choose to modify it. –  chrish Feb 28 '12 at 19:29
show 2 more comments

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.

share|improve this answer
1  
Great answer. Last two lines especially drive home the point. –  Xonatron Feb 27 '12 at 17:44
6  
I think the important rule of thumb is checking if the submethods are orthogonal. If they are independent, can be called in any order and respect the class invariant then its better to keep them separate. If the methods are tightly coupled, break class invariants and/or need to be called in a certain order then perhaps it is better to keep them together. –  hugomg Feb 27 '12 at 18:11
4  
Code complete is full of good stuff. Every programmer should eat a piece of it at lunch every days. –  deadalnix Feb 27 '12 at 18:24
add comment

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."

share|improve this answer
7  
The "80/100/120" columns limit is so we can read the code. Most terminals open at 80 columns, and it's less effort to make the author wrap to 80 columns than it is to make every reader resize their terminal every time they want to read the code. Unlike the other limits, the N-column limit doesn't affect the semantics of the code, it's a purely stylistic rule; in the same category as "use 4-space tabs" or "put opening braces for functions on their own line". –  Dietrich Epp Feb 27 '12 at 17:20
3  
Granted, it's an aesthetic aspect more than a semantic aspect. However, that's where my last sentence applies. It's not uncommon to find Java statements that exceed the 80 column rule with no "nice" place to split the line. This affects readability and understanding of the code. It could also indicate violations of the Law of Demeter –  seggy Feb 27 '12 at 17:33
6  
In the 21th century, my IDE has this feature called "dynamic word wrap". –  György Andrasek Feb 27 '12 at 21:35
9  
@GyörgyAndrasek: But everyone isn't always using your IDE: we look at code in GitHub, in terminals with less, in vim and emacs; and automatic wrapping seems haphazard at best. Coding standards aren't for your benefit, they're for the benefit of people who works in teams. –  Dietrich Epp Feb 27 '12 at 22:40
1  
@DietrichEpp Personally I have a hard time reading code which is always wrapped at column 80. I recently had to work on a Java project where the standard Eclipse formatter was used. And inside a method it was very hard to keep track of lines that went over two or even three lines (I do blame the other formatter settings for that too though). Bottom line is that I think the 80 is a bit outdated. I personally use a limit of 120, have my editors set to enforce that, have a console that is 120 columns wide and Github actually displays 125 columns. And I think it’s much more readable like that. –  poke Feb 28 '12 at 10:08
show 3 more comments

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.

share|improve this answer
1  
+1 for readability. It shouldn't even matter if a method is 20, 30 or even 50 lines long. If DRY and SRP are your goals, it won't matter if the methods seem to be a little on the longish side... although most often you'll find that the more readable you try to make your code, the shorter your methods will naturally become. –  S.Robins Feb 27 '12 at 20:20
add comment

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.

share|improve this answer
show 4 more comments

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.

share|improve this answer
show 1 more comment

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

share|improve this answer
8  
You can't abstract the boilerplate code into its own method? –  erjiang Feb 27 '12 at 15:57
show 3 more comments

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.

share|improve this answer
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
2  
Good of you to chime in, and I think you make a reasonable argument. That said, I don’t agree with your rationale “they're always developers who want their guidelines to be explicit”: you can’t always cater to idiots, and you shouldn’t try. Making guidelines explicit is good so long as this doesn’t render them incorrect: now it’s no longer a guideline, it’s an arbitrary rule. Introducing arbitrary numbers, as you have witnessed, is a dead certain way to get shot down, and justified. –  Konrad Rudolph Feb 28 '12 at 20:37
add comment

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.

share|improve this answer
5  
"I agree with the statement above me" Avoid this kind of sentences in stackexchange sites. Remember that the order the answers are posted is not necessarily the order they are displayed in. –  luiscubal Feb 27 '12 at 20:15
show 1 more comment

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".

share|improve this answer
show 4 more comments

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.

share|improve this answer
add comment

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.. }

share|improve this answer
1  
See also my response to James Andersons's question. In your particular case, I wonder if that validate method is not violating several principles and guidelines at the same time. –  Dennis Doomen Mar 1 '12 at 8:53
add comment

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());   });
share|improve this answer
show 1 more comment

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...

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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