Friday 23 November 2012

Coding expectations

I've been working on kind of cheat sheet for the development team, and I thought I'd post it here too as it counts as geek :)

-->
Pair programming
Pair programming is a technique that hopes to achieve three main things. Those being: better architectural design, shared knowledge and code quality. Pair programming often reduces waste during code review as core concepts are already decided, leading to fewer rewrites.  When done well it helps to build trust and understanding between developers. The following steps are all equally important and can be considered a cheat sheet.

·      Define the task, with small goals in mind. You’ll then be able to move in the same direction quickly.

·      Agree on the solution. It’s worth spending a little time before coding to fix in your minds how your solution will work.

·      Talk constantly about what you are both doing. 

·      The developer with the keyboard writes, while the other one directs. An analogy is that of driver and a navigator. The driver at the keyboard makes the tactical decisions while the navigator keeps them on course and suggests short cuts.

·      Switch roles at least every two hours.

·      Consider checking into a temporary pair-programming local branch in git at each swap point. This shouldn’t be master.

Code Reviews
Code reviews are there to catch mistakes, keep the code base balanced and gain the insight of another pair of eyes before you submit your work to the wider business. It’s there to help and protect you, not to criticize you or force you to change complete direction. When you are reviewing others code, you should be wary of asking for changes to make the code look like you had written it. The following points should be considered during code review.

·      Select a reviewer as close to the code as possible. Therefore if you are pair programming or working with someone on a user story, they should be the reviewer. You must stick with this reviewer for re-reviews.
·      If at all possible, do code review together at a desk before it is even pushed to Gerrit. Even better correct problems there and then. This makes it more immediate, less confrontational, more productive and a better experience all round.

·      Review the code, not the coder. Refer to the code, not the coder.  Don’t say ‘your code here tests the wrong object’ but ‘this code tests wrongObject, not rightObject’

·      The reviewer should assume that things have been done by the reviewee for good reason. The developer has likely given it more thought than the reviewer, so challenges to how something has been done should be diplomatic and open. Equally, the developer may not have thought of everything and may not have thought of the best solution. When such things are raised in review, they should be discussed and either corrected or clarified (e.g. better code structure or comments may resolve the issue).

·      The developer should not take feedback as personal criticism. They should view them as honest questions/feedback aimed at ensuring that the delivered code is of an acceptable quality (fulfills the requirement, complies with best practice in terms of architecture, design and code etc.). Any feedback should be considered openly and as an opportunity for improvement, learning and to deliver a better end product.

·      Be clear in identifying the issue and suggest obvious fixes. ‘This doesn’t work’ is worse than useless.

·      Programming style should be challenged not demanded. Rather than saying ‘This code style is not following a pattern’ say ‘Consider the use of the Command Pattern?’ This however is not an excuse to ignore best practice or consistency issues with the code by the developer.

·      When you review code you become part of that story. If a reviewer just rubber stamp the review, they’re directly responsible for failure. If they sit on a review, or re-review you are responsible for it being held up. Check you requested reviews often.

·      Always respond to every code review comment in Gerrit. Either with “Done” or with a suitable response. This aids re-review.

·      Point out the good. When reviewing code you may learn things yourself or spot elegant solutions. When that happens, say so, the coder may be unsure that it was elegant and it positively reinforces the good stuff.


Checking in
Git is complex and powerful, and there is a lot to say on it, however these general check-in points are important.

·      The code you write is valuable and needs to be backed up as much as possible so that there is no danger of losing it.

·      Check the code in at end of the day. Otherwise you are preventing others from picking up the work to get it finished. We can’t ever predict what will happen from one day to the next in terms of illness or transport problems.

·      More frequent commits mean more flexibility in code reviews. This leads to faster turnaround and less context switching.  Small commits are easier to understand, easier to review, and can be merged to master much sooner. This means that your code gets picked up by others sooner, reducing the complexity of conflict resolution, and ensuring that your code is in use sooner which gives you a higher degree of confidence that there are no corner cases that you missed in your unit testing.