How to make your code reviewer fall in love with you

Code reviews can sometimes be an arduous task for both the person seeking review and the reviewer. But it doesn’t have to be!I have had amazing mentors who held me to the highest code quality standards, which was, to be honest, painful at first — the s…


This content originally appeared on Level Up Coding - Medium and was authored by Sushmita Mallick

Code reviews can sometimes be an arduous task for both the person seeking review and the reviewer. But it doesn’t have to be!

I have had amazing mentors who held me to the highest code quality standards, which was, to be honest, painful at first — the sinking feeling of not getting it right even after the 4th code review. Having a senior developer or peer invest in your growth is something immensely valuable — so we don’t want to take it for granted and make it worth their time.

Recently, I took the time to review the notes that I had made for myself over the several code reviews I have been a part of and through the excellent code quality guides I came across.

#1 rule: The reviewer’s time is golden.

Be your own code reviewer and spot bugs early, so that your reviewer can focus on giving you quality feedback and not lose their heads around indentation errors.

Here are some of the things we can check for before asking a code review:

Clarity

  • The code should be easy to read for someone new to it. You should understand what each line and function does at a glance.
  • The Single Responsibility Principle demands “A class should have one, and only one, reason to change”. Each function should have a single concise intent (whether that goal is processing an API request or just a date-time util function depends on what level of abstraction you are on).
  • Prefer readability over small optimisations. Resist the temptation to squash a paragraph of code into a single line to make it look cool. Making others’ (and your future self’s) lives easier is cooler :)
  • Is code commented out? There are very few cases where this is appropriate.
  • Separate functional and non-functional changes. If your change involved a two line code change, but you formatted your code file to modify tabs / whitespaces and pushed it as part of the same code review — it will be a frustrating job for your reviewer to figure out the actual changes.

Functionality

  • Are there logical flaws in the code?
  • Does the code handle edge cases?
  • Does this change fully address all of the feature’s / issue’s requirements? Is there any ambiguity in how the requirements were interpreted?
  • Is the code you added appropriately thread safe? Is the thread safety and concurrency documented at the class or API level?

Tests

  • Are there unit tests that adequately test the code and have they been run?
  • Is there a code coverage tool being automatically run with the builds? (e.g. JaCoCo)

Abstraction

  • Do not reveal implementation to clients. While a function name should reflect its utility, it should not reveal implementation details which the caller has no use of. More importantly, a client facing API documentation should not expose implementation details. Imagine, if the Google Maps API documentation told told you where they which database they were fetching their data from
  • Are database access operations (DAO) are abstracted into classes of their own?
  • When we design an application, we start with the high level design and then work our way down into the low level implementation. Similarly, our code should also reflect it. Each function should bridge one level of abstraction, calling lower-level functions.

Style

  • The code should follow the style guide native to that language. For example, camelCase is the naming convention in Java, while Python suggests snake_case.
  • That being said, the code should be in line with the style (indentation, spacing, naming conventions, etc) already being used within the module. In some cases, the style guide makes recommendations rather than declaring requirements. In these cases, it’s a judgment call whether the new code should be consistent with the recommendations or the surrounding code.

Documentation

  • Is each function’s intent clear or would minimal comments describing expectations help?
  • Are the public methods and classes documented with Javadoc (Java) or Doxygen (C/C++)?
  • Documenting intent is the most important form of documentation. The biggest question other engineers have when looking at code (or even their own) is, “Why is this code here?”, or, “Why did they (I) do it this way?”, or, “What specific problem does this code solve?”.
  • Write a clear change description — if you are using a tool to generate your code reviews, pushing your code to a separate branch or just sending them your code change over in a zip (yikes!), you should describe your changes in brief. You can think of it of a high level design diagram — but using words.
  • If your change is adding a new feature or involves a major revamping of an existing one, consider having a dedicated design document with to help people get easily onboarded to it. High or Low level diagrams are a bonus.

Reuse and Duplicated Code

  • Is code duplicated within the package? If even a few lines of code are duplicated more than once within a module, it’s a sign that refactoring is needed.
  • Don’t reinvent the wheel. Is there code that could be completely removed or replaced with existing libraries? There is no code like no code!
  • Use annotations wherever possible to keep your code easier to read and bug-free, while establishing a common standard across your team (Eg. Lombok for Java)

Configuration and Constants

  • Are there magic numbers or string constants sprinkled, or worse, repeated, across the code?
  • Does the code have arbitrary constant values that should be configurable?
  • Define variables with modifiers like final (Java) or const (JS) wherever possible. While keywords like final do provide minor performance optimisations, the important human-related aspect of it is about the semantics of the keyword. It signifies that the variable would remain constant throughout the scope and thus improves readability & maintainability.

Dependencies

  • Steer clear of star imports. Only import library dependencies that you need.
  • Are there new external dependencies being introduced that could introduce a new security or operational risk?
  • Are there alternative light weight dependencies that could be used instead?
  • Are you creating dependencies every time instead of injecting or reusing them? The Dependency Inversion Principle states that dependencies should loosely coupled. It requires that a unit of code (class, method, module) should declare dependencies instead of creating them.

Exception Handling

  • Is the code resilient to garbage and null inputs (even from a library that is being called)?
  • Is there a clear and consistent exception handling strategy?
  • Are you logging errors you encounter in a function in a catch block and then throwing it for the caller function to catch again? “Log and Throw” is a known anti-pattern. You can refer this excellent Stack Overflow answer.

Performance

  • Are there signs of over-engineering or premature optimisation?
  • Are there obvious performance or efficiency problems that are trivial to address?

Instrumentation

  • Do we have metrics to analyse the impact of our change? Numbers speak much more than “We saw a big improvement”.
  • Is there an appropriate amount of logging for all levels from error to debug?
  • Does each log message include sufficient relevant info for someone to understand behaviour of system from just reading the log message? Use logging with appropriate identifiers.

General

  1. Break up large code reviews into smaller ones. A good rule is not having more than 150 new lines of a code in the code review. The more bigger the change, chances are your reviewer will need to keep a lot more context in mind while reviewing it and small mistakes might go unnoticed.
  2. Try to write code making use of the latest developments in that language that might improve readability or scalability. For example, in Java, prefer streams over loops. In JS, use ES6 syntax.
  3. Take criticism in a constructive way. A code review is not a personal attack, even if the reviewer’s tone might seem accusatory. Look at it objectively as if you are collectively trying to make the code better.
  4. Automate the boring stuff. Git hooks are pretty cool to automate all kinds of stuff.
  5. Be patient if your reviewer is wrong. Resist the temptation to snap back and instead wonder whether your code lacked in clarity which led them to be confused.

Thankfully, there are many other good coding practices which we can follow to improve our code and our code reviews.

Here’s one of my favourites (and the one that inspired the article name).

I hope your reviewer adores you! All the best.

How to make your code reviewer fall in love with you was originally published in Level Up Coding on Medium, where people are continuing the conversation by highlighting and responding to this story.


This content originally appeared on Level Up Coding - Medium and was authored by Sushmita Mallick


Print Share Comment Cite Upload Translate Updates
APA

Sushmita Mallick | Sciencx (2021-06-03T11:57:04+00:00) How to make your code reviewer fall in love with you. Retrieved from https://www.scien.cx/2021/06/03/how-to-make-your-code-reviewer-fall-in-love-with-you/

MLA
" » How to make your code reviewer fall in love with you." Sushmita Mallick | Sciencx - Thursday June 3, 2021, https://www.scien.cx/2021/06/03/how-to-make-your-code-reviewer-fall-in-love-with-you/
HARVARD
Sushmita Mallick | Sciencx Thursday June 3, 2021 » How to make your code reviewer fall in love with you., viewed ,<https://www.scien.cx/2021/06/03/how-to-make-your-code-reviewer-fall-in-love-with-you/>
VANCOUVER
Sushmita Mallick | Sciencx - » How to make your code reviewer fall in love with you. [Internet]. [Accessed ]. Available from: https://www.scien.cx/2021/06/03/how-to-make-your-code-reviewer-fall-in-love-with-you/
CHICAGO
" » How to make your code reviewer fall in love with you." Sushmita Mallick | Sciencx - Accessed . https://www.scien.cx/2021/06/03/how-to-make-your-code-reviewer-fall-in-love-with-you/
IEEE
" » How to make your code reviewer fall in love with you." Sushmita Mallick | Sciencx [Online]. Available: https://www.scien.cx/2021/06/03/how-to-make-your-code-reviewer-fall-in-love-with-you/. [Accessed: ]
rf:citation
» How to make your code reviewer fall in love with you | Sushmita Mallick | Sciencx | https://www.scien.cx/2021/06/03/how-to-make-your-code-reviewer-fall-in-love-with-you/ |

Please log in to upload a file.



There are no updates yet.
Click the Upload button above to add an update.

You must be logged in to translate posts. Please log in or register.

Related Posts