Code lines per file, methods per class, cyclomatic complexity and so on. Developers resist and workaround most if not all of them! There is a good Joel article on it (no time to find it now).
What code metric(s) you recommend for use to automatically identify "crappy code"?
What can convince most (you can't convince all of us to some crappy metric! :O) ) of developers that this code is "crap".
Only metrics that can be automatically measured counts!
Code coverage has some value, but otherwise I tend to rely more on code profiling to tell if the code is crappy.
My bet: combination of cyclomatic complexity(CC) and code coverage from automated tests(TC).
CC | TC
2 | 0% - good anyway, cyclomatic complexity too small
10 | 70% - good
10 | 50% - could be better
10 | 20% - bad
20 | 85% - good
20 | 70% - could be better
20 | 50% - bad
...
crap4j - possible tool (for java) and concept explanation ... in search for C# friendly tool :(
Non-existent tests (revealed by code coverage). It's not necessarily an indicator that the code is bad, but it's a big warning sign.
Profanity in comments.
Ratio of comments that include profanity to comments that don't.
Higher = better code.
No metrics regarding coding-style are part of such a warning.
For me it is about static analysis of the code, which can truly be 'on' all the time:
I would put coverage test in a second step, as such tests can take time.
Do not forget that "crappy" code are not detected by metrics, but by the combination and evolution (as in "trend) of metrics: see the What is the fascination with code metrics? question.
That means you do not have just to recommend code metrics to "automatically identify "crappy code"", but you also have to recommend the right combination and trend analysis to go along those metrics.
On a sidenote, I do share your frustration ;), and I do not share the point of view of tloach (in the comments of another answers) "Ask a vague question, get a vague answer" he says... your question deserve a specific answer.
Number of worthless comments to meaningful comments:
'Set i to 1'
Dim i as Integer = 1
Lines of comments / Lines of code
value > 1 -> bad (too many comments)
value < 0.1 -> bad (not enough comments)
Adjust numeric values according to your own experience ;-)
Metrics alone do not identify crappy code. However they can identify suspicious code.
There are a lot of metrics for OO software. Some of them can be very useful:
Average method size (both in LOC/Statements or complexity). Large methods can be a sign of bad design. Number of methods overridden by a subclass. A large number indicates bas class design. Specialization index (number of overridden methods * nesting level / total number of methods). High numbers indicate possible problems in the class diagram.
There are a lot more viable metrics, and they can be calculated using tools. This can be a nice help in identifying crappy code.
I don't believe there is any such metric. With the exception of code that actually doesn't do what it's supposed to (which is a whole extra level of crappiness) 'crappy' code means code that is hard to maintain. That usually means it's hard for the maintainer to understand, which is always to some extent a subjective thing, just like bad writing. Of course there are cases where everyone agrees the writing (or the code) is crappy, but it's very hard to write a metric for it.
Plus everything is relative. Code doing a highly complex function, in minimal memory, optimized for every last cycle of speed, will look very bad compared with a simple function under no restrictions. But it's not crappy - it's just doing what it has to.
My personal favourite warning flag: comment free code. Usually means the coder hasn't stopped to think about it; plus it automatically makes it hard to understand, so ups the crappy ratio.
Unfortunately there is not a metric that I know of. Something to keep in mind is no matter what you choose the programmers will game the system to make their code look good. I have seen that everywhere any kind of "automatic" metric is put into place.
Number of commented out lines per line of production code. Generally it indicates a sloppy programmer that doesn't understand version control.
I take a multi-tiered approach with the first tier being reasonable readability offset only by the complexity of the problem being solved. If it can't pass the readability test I usually consider the code less than good.
A lot of conversions to and from strings. Generally it's a sign that the developer isn't clear about what's going on and is merely trying random things until something works. For example, I've often seen code like this:
object num = GetABoxedInt();
// long myLong = (long) num; // throws exception
long myLong = Int64.Parse(num.ToString());
when what they really wanted was:
long myLong = (long)(int)num;
Developers are always concerned with metrics being used against them and calling "crappy" code is not a good start. This is important because if you are worried about your developers gaming around them then don't use the metrics for anything that is to their advantage/disadvantage.
The way this works best is don't let the metric tell you where the code is crappy but use the metric to determine where you need to look. You look by having a code review and the decision of how to fix the issue is between the developer and the reviewer. I would also error on the side of the developer against the metric. If the code is still popping on the metric but the reviewers think it is good, leave it alone.
But it is important to keep in mind this gaming effect when your metrics start to improve. Great, I now have 100% coverage but are the unit tests any good? The metric tells me I am ok, but I still need to check it out and look at what got us there.
Bottom line, the human trumps the machine.
Sometimes, you just know it when you see it. For example, this morning I saw:
void mdLicense::SetWindows(bool Option) {
_windows = (Option ? true: false);
}
I just had to ask myself 'why would anyone ever do this?'.
At first sight: cargo cult application of code idioms.
As soon as I have a closer look: obvious bugs and misconceptions by the programmer.
TODO: comments in production code. Simply shows that the developer does not execute tasks to completion.