Ask HN: How to improve my code review skills? | Hacker News Reader

Ask HN: How to improve my code review skills?

25 pointsnumbers6 comments

Hello HN, I know this is a vague question but for 2018, one of my goals is to improve giving feedback. Feedback as in technical code reviews and also feedback to my teammates and managers.

There's a lot about how to improve giving feedback face to face but I've always struggled giving thorough code reviews and I'm looking for some suggestions and maybe techniques I can employ to give worthwhile feedback. Currently, I feel like most of my feedback is "oh you should explicitly check type here" or "has this been tested with a particular kind of user?" and nothing past that.

Any help is appreciated!

View on HN

To gain some context, I would suggest reading the Legacy Code book by Michael Feathers or read about Kent Beck's 4 rules of simple design, or even read the Clean Code book.

I think the first thing to consider in doing a review is first look to see if the code is easy to read and understand. There is that number out there that states we read code ten times more than we write it.

IMO the mindset to be in when reviewing code is not one of evaluating whether it's correct, or even if it's written the way you would write it.

Instead, ask yourself "how would I use this code?" How would you call it? How would you build on top of it? How would you maintain it?

If those things aren't clear, that's where to start asking questions and raising concerns.

1 - Always try to think of corner cases while reviewing code - where it could throw exceptions etc.

2 - When finishing the code review, try to think holistically about what it's trying to do and whether it's doing it in the right way or not. Check the level of abstraction where the code is being added/removed and think whether it's right or not? Or whether it should be at some other level of abstraction?

3 - Any security concerns that might arise with the new code that's going in?

4 - Anything which involves a call to a network / filesystem can fail? Check if those cases have been handled properly.

5 - Are variable names, function names, class names descriptive and yet concise? Do they follow your company's coding standards?

6 - Try to think from a perspective of "single responsibility principle".

7 - For all the new flows, replay each flow in your head and try to see if it can be optimized or not? Can we make it faster? Will the code that has been added/removed degrade performance to an unacceptable degree?

8 - Always be on the lookout for logical bugs. Never assume that the coder will have done it right. Always be critical.

9 - Last, but not the least, take out proper time to do the code review and think deeply about the code that's going in.

Before rewiewing anyone I speak with them so we have an understanding on what kind of review they want : - tone (harsh, civil, encouraging, etc) - pedantic level (grammar nazi, normal) - incremental (fix this first then we'll see what's next) or all at once (fix this and this and this) - technical area they currently want to improve - common mistake they make I should be watching out

And I also tell them what I want when they are reviewing me.

I aim for my feedbacks to be effective, so I don't want the reviewee to lose time because he/she feels hurt or my language wasn't tailored for them

Presume that the next dev to be hired will be the founder's slow nephew. Make sure none of the conditionals are too hard to read. No nested ternary operators, no bullshit variable names, no magic numbers. That's like rule 0 stuff. Past that I think it depends more on language and conventions and stuff.

To me code reviews start with coding standards/best practices documents which would contain a source or justification.

if you come across something not in it then add it. Maybe review it over time and if someone new starts on the team.

also, should be team effort so theirs buying. someone needs to make a final decision or maybe team vote depending on how team oriented or top down you want to be.

Reply on news.ycombinator.com