Start reviewing your code, seriously

Code review is one of the most important (and overlooked) activity in the team's life as it (should) guarantees the "right" level of quality for code that is going to be production code.

Point is: what to review, business logic? unused variables? code style? and who should join the review?

1 - Who: Code review is yet another chance to learn by looking at real world code with real feedback from team members that master the language and the business logic under test. So in principle anyone can join. Realistically not any team member and not any time should join but observers should be encouraged. Remember, on a review there are no silly questions JUST silly answers (although there might be silly comments)!

2 - Test: best way to learn what a snippet of code is doing is by checking its tests. No tests? no review ... 

3 - Design: Start by checking that the code under review adhere to the SOLID principles. (e.g.: Why we don't have an interface -or an abstract class- here? this class looks like has two responsibilities, etc ...)

4 - Code Simplicity: it is really hard to implement features with simple solutions and engineers tent to make solutions over-complicated. Don't, if not needed. e.g.: Do you really need that multi inheritance here? do you really need a thread here, why you didn't use the 'async' instead that allows you to abstract on a lot of low level details? (C++ stuffs), etc ...

5 - DRY: this code looks familiar! "oh, yes it is the same algorithm I've just reused it!". Well, although this is a way of reusing code the preferred way is another: write once and in one place :-)

6 - YAGNI: see point 4. Don't predict the future, if a feature is not needed don't over-complicate your task. In this case what you've written is "just your guess" and not what a customer asked (and payed) for.

7 - Exceptions: if one of the above points has not been followed and there was an intention and a clear and accepted reason => fine, mark the point done and go on!

8 - Boy-scouts: "Always leave the campground cleaner than you found it." (suggested reading: Things-Every-Programmer-Should-Know) if you find a piece of code that might be easy to improve => just do it!

9 - Coding conventions: the team should have a well defined (and accepted) coding style conventions (which includes space, braces positions, etc). The review should not highlight any of these issues as it is symptom that is a manual activity and, honestly, nowadays there is no reason why it should't be automatized. Goal is to keep the review focused on the stuffs that really matters!

10 - Pair: why not pair-reviewing the code?

If a team member highlight an issue (e.g.: point 9) just ask to the team to openly discuss it (retrospective?) and see if there's a way to improve/fix it (e.g.: I propose this code style for these reasons, I propose this tool that automatically check ... etc ... ).


要查看或添加评论,请登录

社区洞察

其他会员也浏览了