Personal blog

The missing piece of (almost) every code review

I might be the only developer in my company enjoying code reviews (from both sides, actually). On reflection, the main reason for that is that they make a great opportunity to learn something new. Which is cool, even if it’s someone else’s point of view on programming.

The years of education, the number of conferences, the whole knowledge, and experience. All this is now concentrating on something very, very concrete – the feature to implement, the code to write.

As a reviewer, I don’t only point mistakes. I like to ask questions about the decision making or simply talk about the code. Perhaps I prefer talking about the code than coding itself, which sometimes scares me.

Anyway, out of multiple discussions during code reviews, I found out that most of them lack one piece, which is…

Idea review

That is, a short discussion with a potential reviewer after you have an idea how to implement the thing, but before you start writing any code.

I have read a few guidelines about code reviews (you know, passion…) and I have never encountered this idea on any. I learned this term in my ex-company, maybe it was even coined there.

Perhaps idea reviews are not considered as a part of the code review, but I think it very much fits it.

Why review ideas?

There’s the rule saying: the further in the process, the harder and more expensive it is to fix a bug. It’s praised to catch bugs and mistakes as early as possible:

  • when in production – that’s the worst what can happen. Not only the end users could be affected. Also bug fixing would need to take the whole development and deployment loop to be included in next iteration.
  • when in “regular” code review – sounds much better. Not a single line of code has been executed on production so far. You can quickly push the fix.
  • when in your own development loop – that sounds even better. You can deal with the flaws of the code on your own, never bother other people with reviewing the incorrect implementation.
  • then why not push it even further and try to find as much as possible before writing any code?

When the code is already written, and the effort is already put, it simply cannot be undone. It’s hard to get back to decision making when the decision is not only made, but also implemented:

  • I would do this in different way. My solution is slightly more maintainable and elegant.
  • Yeah, but you know, my slightly less maintainable and less elegant solution is already written. Yours would take next 4 hours to implement. Let’s not waste time, please approve.

Sounds familiar?

The idea review is literally the exemplification of thinking twice before writing any code. This 5 minutes of talking could save you from writing code again in the future.

The importance of idea reviews

From my experience, people consult the ideas only on very rare occasions, for outstandingly hard or complex tasks. Whilst I would suggest doing it the other way around. Let’s have a quick idea review on default, and let’s skip it only if there’s a good reason for that, for example:

  • The idea review would take more time than the task itself.
  • The task is too complicated to explain in relatively short time.
  • The task regards something already discussed multiple times.
  • The task is too straightforward and you’re not going to clean up the code.

Following reasons are not a good enough justification:

  • Lack of time – seriously, we’re talking lack of 5 minutes.
  • You have no idea of how to write it – in this case, the discussion is even a must
  • The task is simple – see below why.

The reason why I would like to discuss even the simplest tasks is the boy scout rule – Leave your code better than you found it. Ok, the task could be straightforward, but is the way of cleaning the code as straightforward?

  • How can we follow up to changing the color of a button? Introduce new CSS class? Extract color variable in SASS? Create a new component (in case of React or similar component-based framework)?
  • Are you sure you picked the right choice? Are you sure enough to not even talk about it for 5 minutes to your teammate?

In short, this is how I imagine idea reviews:

Goals

  1. To make sure the developer understands the scope
  2. To make sure the developer didn’t miss potential solutions
  3. To make sure the developer picked up the best solutions out of available ones
  4. To introduce the reviewer to some clever idea, if you’re going for it

Rough sketch

  1. It happens before writing a single line of code – by definition.
  2. It’s short (up to 5 minutes) – this is very important and very hard, due to the next point.
  3. It’s kept on the high abstraction level – save the details for the actual implementation.
  4. It’s more like a chat than a monologue – a question or unclearness is an opportunity to learn something new. Don’t hold them back.
  5. It happens face-to-face – written idea reviews miss the point of revealing the thought process.
  6. It’s OK if the review only confirms the idea

Things to cover

  1. Quickly explain the task
  2. Potential solutions
  3. Alternative approaches and why they are worse than the one you picked

From my experience, the most difficult part is to introduce the task to the reviewer and not get into much detail. I would rather end the discussion having some open questions than prolong it. Suppose the idea review to out only the most obvious flaws.

2 Comments

  1. Szymon Fiedler

    > This 5 minutes of talking could save you from writing code again in the future.

    Meh. 100 hours of coding can save you 1 hour of talking.

    • Bartek

      That’s my kind of save!

Leave a Reply to Bartek Cancel reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.

© 2024 Bartosz Krajka

Theme by Anders NorenUp ↑