As some of my readers already know I am a big advocate of code reviews. However I must admit that my post about proofreading your code is just one step too fast.
Why? Well, as Frans Bouma points out you should think first, ‘doing’ is for later. Let me explain what I mean with an example, it is something that happened in our team for some time.
In our team, software developers work on issues from an issue list. The issue list is full with registered features and existing bugs, and as team lead I can simply assign every issue to any of the software developers. Now, if some software developer tries hard to be productive than what she wants is to churn that list of issues on her name down as fast as possible. Which of course is a good thing.
However, in our team it turns out that some developers take on the following approach for working through issues as fast as possible:
1. Developer takes up new issue: Code the feature/bugfix
2. Assign the work for review to a peer.
Which, in our current work process is exactly the thing they are supposed to do. This is because every issue that needed a code changes has to be
proof readed by a peer..
However it is missing a very important step. Why? Because sometimes the work process ends up like this:
1. Developer takes up new issue: Code the feature/bugfix
2. Developer assigns the work for review to a peer.
3. The peer reviewer bounces the issue back with some comments to use a better algorithm.
4. Developer implements the new algorithm and assigns the work back for review to peer.
5. Peer signs of the work and puts the issue ready for test.
But… look this over! WTF Happened?
You might say that it was the software developer doing the code ‘thus the work’ but who is doing the thinking here? It’s the peer in the role of reviewer, not the developer.
It turns out that not the developer is doing the work, but the peer is doing all the hard work!
And that is the problem. As Frans Bouma points out you should think first, ‘doing’ is for later, and I quote from his page:
people should focus on thinking. In the Netherlands we have an old saying: “Bezint eer ge begint”, which translated to English is something like “Think everything through before you start”. ….
I’d like to line out how I write my software, how thinking is an essential part of every step I take in the whole process and will illustrate it with an example which hopefully will illustrate that some extra time spend on the thought process before writing any code is very valuable.
Frans Bouma describes the six steps of implementing new features, I have deliberately summarised it a bit:
1. Think first. ‘Doing’ is for later.
2. Analyze what you need based on step 1.
3. Document the results of step 1 and your analysis result from step 2: write which alternatives you’ve investigated and which one you’ve picked and most importantly, why.
4. Prove your algorithms first. This is not that hard and doesn’t require any code
5. Implement algorithms / data-structures as designed.
6. Test the implementation of what you wrote.
As the algorithm is already proven, the tests prove the implementation. If you want, you can also start with this before ‘Implement algorithms’ by implementing it using mocking and tests. That’s not the point, the point is that the code is the end-result, not the start.
Of course proofreading and code reviews are still there in step 5.
Is this implementation correct? …go to the code to see if we made a proper projection of the step to the executable form: the code. If not, we have to change the code, not the algorithm. Don’t think lightly of this and be careful not to cut corners by ‘assuming’ it is OK. A human isn’t a very good source code interpreter but the developer of the code should at least try hard to check whether the implementation is indeed how it should have been. That already will catch obvious bugs and mistakes, as the algorithm is correct so we have to worry only about the implementation.
..
This is also the place where code reviews come into the picture and the reason why they work: other people will look into how an algorithm is implemented exactly, and as each person will actually make the same projection from the algorithm to code, any difference in what they would have done themselves vs. what they are reviewing will trigger discussion and will in the end result in better code.
The most important thing to learn here for software developers is that if your team does code reviews, this does NOT mean you can depend on the code reviewer(s). Thinking is the job of the software developer, not the reviewer.
Frans Bouma starts his post by saying:
I’d like to line out how I write my software, how thinking is an essential part of every step I take in the whole process and will illustrate it with an example which hopefully will illustrate that some extra time spend on the thought process before writing any code is very valuable.
And valuable is exactly the point. Pinball wrong implementations of code to reviewers with your big flippers will only result in TILTing some reviewers who will bounce issues back to you more often. And that is not only hurting the process, it is hurting your value.
Comments
2 responses to “Software developers: think first, then proofread your code.”
Interesting post Melle. What intrigues me is why developers would exhibit this behavior: Why would it be preferable to bounce your work a reviewer rather than think it through once and get it done right the first time?
Some explanations off the top of my head:
– pedantic reviewers: If the reviewer will come up with demands to redo your work, regardless of the quality of those efforts, then you might as well invest as little time as possible before requesting a review.
– complicated test suites: if it takes a lot of time/effort to set up a suitable test for your fix, then the developer may choose to just spin the wheel and see what the reviewer comes up with. If the reviewer sets up the testframework himself to review the code, then the developer has gained even more from his ‘lazy’ behavior.
– optimizing what you measure: if the time a developer takes before requesting a review is used in a significant metric in some way, then developers will have an incentive to reduce the time-to-review. This may be aggrevated if a review can result in a new issue rather than the old issue being kept active (thus creating more issues for the developer to fix quickly)
there are certainly more possible explanations, but I can’t come up with the right now 😉
cheers,
Mathieu
I agree what you are saying, but isn’t it a fact that in most companies the “reviewer” has the overall knowledge of the implemenation whil a developer only know a small part of the application. which could lead that developer in his own “thought-process” comes up with his/her correct algorithm for that part?