After I write some code, I always test and inspect it thoroughly. I’ll write Unit Tests, running manual tests, constantly reading and re-reading the code. Any slight change I make, I then end up repeating the process. Even when I’m about to send it to review, I get paranoid that I’ve missed something, so end up looking at the files again to make sure.
It’s often rare that I get any (serious) feedback to change my code, and it’s rare that Testers will find anything wrong with it. But then sometimes I wonder if I’ve spent a little too long in the development stage.
However, Colin and Derek seem to take the opposite approach and end up coming out with something more quick and dirty.
Derek always seems to come up with a solution close to a hack which I’ll tear apart in the Code Review. After, it will end up being rejected by Testing and he will put in another bodge.
Colin spends time writing Unit Tests but often misses some scenarios. His methods often need refactoring down more, and methods/variables/classes may not have the best names. His solutions still come across as rushed, and he seems to cut more corners when the deadline approaches. When I point out his mistakes, he often says “the deadline is tomorrow night, so I just have to get it checked in”.
But surely it’s useless if it doesn’t work? In a similar fashion to “it’s better late than never”, you could say “it’s better that it is late and works, rather than getting it early and broken”.
He got some comments last week to refactor some of his methods. He quickly changes them and submits his new method for review. It was a one-line method, and somehow there were 3 mistakes in it. The thing is, he had Unit Tests covering every possible scenario of that method; he just didn’t run them. Unit Tests take a several seconds for a large batch of them to run; why would you cut corners so much that you don’t even run the unit tests? If you had full coverage, it will allow you to cut manual testing, but to cut testing completely? Madness.
Anyway, his code was something like this:
public string DisplayInfo(bool isScotland)
{
return DisplayInfo(Configuration.Scotland==Configuration.Scotland);
}
The 3 things that are wrong:
- he is passing in a parameter “isScotland” then not using it
- where he should be using “isScotland”, he is calculating it…but wrongly. He is comparing a value to itself; so it always evaluates to “True”
- The method just calls itself, which calls itself, which calls itself…until it crashes with a StackOverflowException because it’s an infinite loop.
Absolute Junior-level coding.
Stranger Coding Tings 