When people who are afraid to show their own ignorance avoid PairProgramming as if it were hell. Others do not like PairProgramming because it is harder to be a slacker and browse the web while PairProgramming.
An InformalCodeReviewBeforeCheckIn is good enough in this case. I learn what other people know, they learn what I know. It takes time to learn, but InformalCodeReviewBeforeCheckIn takes only 1% to 5% of the development time. It has a high ReturnOnInvestment, considering that the investment is my time and the return what I learn.
- How are the costs and benefits cited above determined? Where does the 1% - 5% range come from and how was it determined? What is the benefit used to determine that the return on investment is high? Frankly, I believe the 1% - 5% cost is way too low and the implied benefit is probably high.
- Most informal code reviews take less than 2 minutes for a whole day of work. Some programmers are great for coding fast, those need to run PMD before code review, to reduce the CopyAndPasteProgramming.
- Some reviews take days. Specially for developers who write too fast or who are not careful enough about their test cases. You can spend as long as 3 continuous hours refactoring code and yet need some massive rework that eventually even break the tests. You review the code again and you see a big bunch of new problems. This may continue for several days. I recommend dividing tasks in that case. Grab the task in the TaskDatabase and divide it in at least 3 tasks. Grab the task that is already finished and the code looks ok, check it in. Then grab the next one, if something is not right, make the small change now. ReFactor it. Check it in. Now grab the hard one. Ask the developer to do the same: Either fix it in a few minutes or divide it. -- GuillermoSchwarz
- I doubt the 2 minute number for a review. Let's assume just 1 other person for a review. We'll allocate 1 minute for getting the person and having him come to your office. This leaves 1 minute to split between 2 people or 30 seconds for the reviewer to come up to speed on what is being done and review the code, and for the writer to implement any changes. To be a little more generous, 1% of an 8 hour day is just under 5 minutes. If one can get a second person within 1 minute, that still only leaves 2 minutes per person for the review. At 5%, we have a 24 minute budget, we can allocate 4 minutes to get a reviewer (this seems like a more reasonable, but low number), and have 10 minutes per person remaining for the review. This still seems way too little time for anything more than a cursory review, and even then falls apart if one checks in code twice daily. Relying on reviews is simply not cost effective.
- It is done the opposite way. He marks the task in the TaskDatabase (for example Insecticida) as ReadyForCodeReview?, the manager gets an email indicating so while the developer can continue to work in other tasks. The manager finally shows up in the developers office and the developer starts AraxisMerge? or another folder comparison tool, to show the changes against the code in CVS. Sure it took a lot of time to write that class, its unit tests, debug it, etc., but a class can usually be read in a few minutes. The developer needs to explain it, so it is even faster. Now that's assuming that the developer wrote the whole class from scratch. Reading a new may take anything from a few minutes to a few days, depending on how complicated the code is. This encourages to write simple code that works and is readable. When developers are asked to organize code in small classes (FewShortMethodsPerClass) it becomes simple to do the code reviews.
- Some code reviews take 3 hours or even days. Ask developers to run PMD first to remove duplicated code. It will take them days to remove duplicated code without removing functionality. This is a GoodThing. The worst that may happen is that your code repeats itself over and over, so that whenever you want to change anything, you always forget to change something. This is the source of most HeisenBugs.
- If you manage 10 people, you will be doing reviews all day and you will be always behind, because it is 5% of the time that the developer used to produce the code that you will use in the review, on average. Team leaders should review the code because they have more experience, but if an experienced developer is not available, a rookie will do the review just fine. Later of course, using something like CvsWeb, the team leader will review the code in his spare time. He will find design bugs and add them in the TaskDatabase for others to fix. Remember the code compiles and passes all UnitTests because of ContinuousIntegration, so probably you won't need to take out the code from CVS.
- The initial question still stands. Where does the 1% - 5% cost range come from and how is the time allocated? The benefit side still provides even less quantification; what is the value provided?
This is very similar to ReviewBeforeCheckin, but here the review is not formal. Formal = Written + Public.
Informal Code Review Before Check In