views:

313

answers:

6

When is a function too long? is a subset of this question, I think.

What are a few good metrics for determining that a class is too long?

I'm rerevising a set of code acceptance guidelines for a project with external contractors, and realized I haven't covered this in the past, but should cover this in the future.

+28  A: 

When it has more than one responsibility.

Let me quote Robert C. Martin's Clean Code here: "The first rule of classes is that they should be small. The second rule of classes is that they should be smaller than that. ... With functions we measured size by counting physical lines. With classes we use a different measure. We count responsibilities." [Chapter 10, page 136]

Steven
+1 top answer sir.
Matt Joslin
At your service ;-)
Steven
This is obviously not true. Have you ever used CRC (class, responsibility, collaborator) cards - one of the classic tools used in OO design? Each card has a single class and a list of SEVERAL responsibilities on it.
anon
@Neil, I would argue that OO design methods from the 80s do not incorporate the large body of experience that has been gained since then. Still the concept of these cards seems to be useful, so thanks for mentioning them!
Thorsten79
@Thorsten It's an oldie but a goldie. But even a class as simple as TCP/IP socket has more than one responsibility, unless you make the responsibility as vague as "be a socket", in which case the whole responsibility concept becomes worthless.
anon
@Neil: Well that's the problem with boundary cases. There is only so much you can take away before making a class sclerotic. IMHO this is where experience kicks in. I do code reviews with our full-time architect, but I don't give in to ALL the architecturally appealing advice he has. :)
Thorsten79
That's a really good - and concise answer. +1 and a checkmark.
Dean J
+2  A: 

One class should have only one responsibility. That is a better measure than its length. So, when designing your code, every unit of your design (a type or a class) should be responsible for only one thing (whatever "one thing" is in your case). If you keep it as simple as possible, you won't get in a mess.

anthares
+1  A: 

When you think that now it has become harder for you to manage it and gets you stuck.

Sarfraz
I'm trying to come up with a standard for others to follow; I have a few 3000+ line classes, and while I want to explain to them this is a bad idea, I'm trying to set down rules to prevent me having to act as a backstop in the future.
Dean J
+1  A: 

Ignoring the design pattern in use, I would consider the scope of responsibility the class fulfills. If the scope is too large, it should be broken up into specific responsibilities, abstracted away, or made more generic.

I wouldn't actually take the number of lines as a meaningful metric.

Dolph
Not at all worried about "lines of code" as the metric, just trying to find more meaningful metrics to take it's place.
Dean J
+1  A: 

No more than 17 lines. No more, no less. So if is under 17 lines carriage returns will do the trick. If its more than 17 you will need to start calling other functions from within the function.

For example:

public function myFunction() {
...
line 17: myFunctionPart2();
}

public function myFunctionPart2() {
...
line 17: myFunctionPart3();
}

And so on.

Its pretty standard programming practice.

wowcat
He was asking about classes... not functions. 17 lines in a class would be less then tiny.
Matthew Whited
Oh, for classes the limit is 19 lines.
wowcat
+1 because this is funny.
Steven
+1 for knowing the True Secret to Programming, the Rule of 17. Note here that if you add 25 to 17 you get 42, and we all know what that means.
Dave Sims
+3  A: 

Class fan-out complexity: The number of other classes a given class relies on. Also the square of this has been shown to indicate the amount of maintenence required in functional programs (on a file basis) at least.

Cyclomatic complexity: Checks cyclomatic complexity against a specified limit. The complexity is measured by the number of if, while, do, for, ?:, catch, switch, case statements, and operators && and || (plus one) in the body of a constructor, method, static initializer, or instance initializer. It is a measure of the minimum number of possible paths through the source and therefore the number of required tests. Generally 1-4 is considered good, 5-7 ok, 8-10 consider re-factoring, and 11+ re-factor now!

Sergey Teplyakov