Don't get me wrong, I'm all for using less-than-perfect code as an opportunity to both learn and teach about testing, refactoring, cyclomatic complexity, clarity, etc. However, there are some times when even I must shake my head and say "WTF?!?"
Here is an example of some code that precipitated one of those moments:
That thought begs the question, however, of what would I say/do in a code review with someone who apparently doesn't know that equality and inequality operators (among others) returns a boolean value? Would it be as simple as pointing out that fact, or would it require addressing more serious deficits in understanding?
How would you go about addressing code like what's above, or what's out there on HatePaste, in a code review with the developer who wrote the code?
2 comments:
It's not easy to make such a code review very constructive, I've done exactly such a "code review" before and it was a failure.
If I'd do it again I'd probably ask two questions
1. What type does foo == bar evaluate to?
2. Do you know what accidental complexity is?
The latter in order to initiate a discussion about it's importance in software industry.
I notice there are unusually few comments, any other helpful strategies?
I share your pain. I've had many-a-code reviews and design reviews that went down the same path. Some people just aren't cut out for real computer science, IMO.
Another personal favorite is the following, using LINQ:
foreach(var foo in fooList)
{
var matchingBar = barList.Single(x => ...);
...
}
If you did this in TSQL, you'd probably be fired. Yet, somehow, this is acceptable in LINQ.
Another example:
foreach(var foo in fooList)
{
var myFooKey = foo.StringProperty1 + foo.StringProperty2;
var duplicatesExist = fooList.Any(x => x.StringProperty1 + x.StringProperty2 == myFooKey);
...
}
Post a Comment