Code Smell Metrics

For a recent posting on the ExtremeProgramming list, I put together this list of "metrics" I use to detect code smells. This list might also be helpful for people new to refactoring, to begin to understand how to detect poorly factored code. All of the specific numbers below must be tuned to your own tastes and coding style, of course.

I'm sure there are more, so feel free to add as many as you can think of. This list is VERY roughly in order from most smelly to least, so please try to preserve that if you change this list in place.

• Sections that are untestable, or hard to test (Emination of odors that follow contribute to untestability, also see CodeUnitTestFirst)
• Duplicate code fragments (no matter how small) (See OnceAndOnlyOnce)
• Comments, or places where a comment would be helpful (See ToNeedComments)
• Poorly named classes, methods, or variables (See MeaningfulNames, BadVariableNames)
• Complex if statements (or other statements)
• Methods with more than 20 (?) lines of code
• Methods with nesting more than 2 (?) levels deep
• Methods that require more than 3 (?) parameters
• Static classes ("class" with only static methods)
• Singleton classes
• Classes with more than 10 (?) public methods
• Classes with more than a couple private methods
• Classes with more than 2 (?) member variables (unless the class is really a wrapped data structure)
• Classes with several get/set method pairs (unless the class is really a wrapped data structure)
• Classes with inner classes nested more than 1-2 deep (I found one 6 levels deep once...)

More:

• Any global or static variables
• Boolean parameters to a method
• Cyclic (two-way) dependencies between classes

- KevinSmith

I don't know if any of these qualify as metrics. Most seem to be trigger points rather than measures of anything.

---

I'm not at all sure these qualify as metrics as opposed to indicators

You are correct. Many are metrics. Some are not. If you want to move all of this to CodeSmellIndicators?, feel free. KevinSmith

I'm not sure any of these are really metrics. They are great inidicators, but BooleanMetricsAreUseless.

---

frex:

• Sections that are untestable, or hard to test
  • How is "hard" defined?
    • for example when your code upon reception of particular parameter consults that with few other tasks or even remote machines. imagine you have to write code that would simulate that remote machine to indicate failure or so. 'hard' is anything that requires more code to begin validation than the actual code that the function already covers and uses (that's my metric). if i cannot test code with a suite executed by equally small test program, i consider splitting that apart

• DuplicatedCode fragments (no matter how small)
  • What's the minimum size (lines? statements?) for something to be a duplication? Should I refactor if I have 2 assignments to i?
    • See DuplicatedCode.

• Comments, or places where a comment would be helpful
  • Certainly the latter, but comments alone are often intent descriptions, instead of detailed explanations.
    • not really. imagine a function that calculates a power of value n. then comment says a whole lot of detail about the function that you don't care about, is redundant, not required, give details that are not covered or misleading. now funny thing: some people actually read comments rather than code. so what would happen if your comment said this function calculates Epsilon rather than Pi? (sum, rather than multiplication)

• Poorly named classes, methods, or variables
  • What definition of "poorly names" is used?
    • int a,b,c,d,...,k; k=p1*p2; a=k>>1+7; while (--a) b+=(k<<1)-3; return z;
    • what a 'poor name' is - is up to you and the standards you follow. you can instruct your people to include at least the most important role of a variable in its name to make it something that looks more understandable. as with the other metrics - you set the bar.

• Complex if statements (or other statements)
  • How many clauses in a complex if?
    • if logic cannot be fit in three ands/ors, then use carnaugh table. if you dont know how, and your logic is excessive, it smells. of course this is my metrics, you could set your own.

Metrics need to be measurable, which means that they need to be unambiguous.

• the metric only depends on your standards. same as fault containment. So what is fault containment? does it mean the code has to be 100% error-free? good luck with that. some companies follow six sigma methodology, others believe in faults per kloc measurements, where number of faults allowed is carefully chosen by the company. and you have to obey these numbers. it's not something that is hard-coded in universe constant.

For those of us that can't get past a quoted use of the word "metric", I've added links to other discussions that may help illuminate the issues a bit more. I also added the entry about the static class to the list. -- TomKubit

Added few more notes - T. Wiszkowski

---

I'm sure that some people have scripts for DetectingCodeSmells?. Would anyone care to share? -- BenA

See DetectingCodeSmellsInPhp. -- BrentNewhall

---

CategoryRefactoring