9 Effective Code Review Tips

9 Code Review Tips

For everyone:

  • Review the right things, let tools do the rest

You don’t need to argue over code style and formatting issues. There are plenty of tools which can consistently highlight those matters. Ensuring that the code is correct, understandable and maintainable is what’s important. Sure, style and formatting form part of that but you should let the tool be the one to point out those things.

  • Everyone should code review

Some people are better at it than others. The more experienced may well spot more bugs, and that’s important. But what’s more crucial is maintaining a positive attitude to code review in general and that means avoiding any ‘Us vs. Them’ attitude or making code review burdensome for someone.

  • Review all code

No code is too short or too simple. If you review everything, then, nothing gets missed. What’s more, that makes it a part of the process, a habit and not an afterthought.

  • Adopt a positive attitude

This is just as important for reviewers as well as submitters. Code reviews are not the time to get all alpha and exert your coding prowess. Nor do you need to get defensive. Go into it with a positive attitude of constructive criticism and you can build trust around the process.

For reviewers:

  • Code review often and for short sessions

The effectiveness of your reviews decreases after about an hour. So putting off reviews and doing them in one almighty session doesn’t help anybody. Set aside time throughout the day including breaks not to disrupt your own flow and help form a habit. Your colleagues will thank you for it. Waiting can be frustrating and they can resolve issues quicker whilst the code is still fresh in their heads.

  • It’s OK to say “It’s all good”

Don’t get picky, you don’t have to find an issue in every review.

  • Use a checklist

Code review checklists ensure consistency – they make sure everyone is covering what’s important and avoid common mistakes.

For submitters:

  • Keep the code short

Beyond 200 lines, the effectiveness of a review drops significantly. By the time you’re at more than 400, they become almost pointless.

  • Provide context

Link to any related tickets or the spec. There are code review tools that can help with that. Provide short but useful commit messages and plenty of comments throughout your code. It’ll help the reviewer and you’ll get fewer issues coming back.

A Guide to Open-Sourcing Your Project at Work

Congratulations! You’ve written something at work that is amazing and you want to share it with the world! This guide covers three key areas that you should consider before making the leap: Why, when and how to do it.

Why Should I Open-Source My Work Project?

Open-sourcing your project at work can be a great idea. It can:

Help you build a developer-friendly brand

  • From those with a developer-focused product, like Stripe and Twilio, to those with APIs, like Facebook, Google and Square. Open-sourcing your code can be a good way to build your company’s relationship with developers.

Allow you to give back to the community

  • Just think of all the libraries and software you use on a daily basis that make use of open-source code. Adding your own is a good way of paying it forward so that others can benefit from your contribution. We’ve open sourced a number of libraries and even whole products.

Help you to recruit

  • Take Yahoo and LinkedIn for example. They’ve found that through their commitment to Open-Source projects (like Hadoop and Kafka), that they’ve been able to encourage developers to join them who otherwise might not have.

Gain more contributors than your project ever would have in-house

  • Like for example Square’s Dagger, a dependency injector for Android and Java. Having released it, many developers are contributing to it, including those at Google. In fact, Google developers have been contributing more than Square’s developers do themselves.

When Should I Open-Source My Work Project?

There are two conditions that you would want to meet before open-sourcing your project. You want to make sure that:

It won’t hurt your business

  • It may be an impressive, complicated bit of code that would be useful for other products beyond your own. Yet if that development is your secret sauce, then giving it away would be bad for the business. Likewise, if your library is an integral part of what makes your product unique or even what makes it possible, then you might want to keep it in-house.

Your code is helpful to others

  • Consider whether anyone else would actually want what you’ve created. Is it so uniquely tied to your workflow or infrastructure that it wouldn’t be useful for others? As a rule of thumb: if making it suitable for general consumption would make it less useful for yourself, then it’s probably not worth the effort.

Ok, so you’ve met those two requirements. Then let’s move to the mechanics of open-sourcing some code.

How Do I Open-Source My Work Project?

Step 1: Audit your code for security leaks

  • Chances are higher than you might like to admit that you or a colleague have left some passwords, usernames, IP addresses, machine names, personal contact information or other security hazards somewhere in your code. Keep in mind that this applies not only to your final master code but also to all the changesets you’ve had in the past.

For that reason, we recommend you do two things:

1. Make a brand-new repository
    • Chop off all the history of the code up to that point. There will be a new history and it saves you having to audit all the historical versions of your code. Plus, no one needs to know that it took you two weeks to wrap your head around C++11 lambda syntax.
2. Audit the code for security problems
    • This will take a lot less time than you think. Look especially at test suites and any places that are near connection points to other systems.

Step 2: Strip your code of profanity and immature pot-shots

  • While you’re in there, also rip out anything inappropriate that makes you sound more like a teenager than a professional. This doesn’t mean you can’t have any humor in your source code. But it does mean that jokes made at the expense of your competitor, a customer or the decrepit browser you’re forced to support might not be appropriate.
  • If in doubt, think about whether you’d feel comfortable reading your code loud to those beyond your team.

Step 3: Make sure your code adheres to best-practice naming and formatting

  • You’ll want your open-source code to be examples of your best work. Make sure you are using good, standardized naming conventions and formatting. Use tools like pyflakes/pep8, jslint, gofmt, ReSharper and others to help.
  • Also, keep in mind that if you’ve been wanting to do the One True Naming Standardization for your project, now’s a good time. Once you open-source your code, there will be a lot of inertia to avoid breaking changes. Get those done before you release. It’ll also make it easier for other contributors to get started with your code.

Step 4: Document it

  • You don’t have to write ninety pages of info docs but you should at least have a nice Markdown-formatted README.md in your root directory that explains what your software is, how to use it, and (if applicable) how to build it.
  • If you’re releasing a library, you should also make sure your code has docstrings/JavaDoc/whatever so that you can generate API documentation.

Step 5: License your code

  • You may want to get some proper legal advice on this. But before releasing your code, you should pick a license. Unless you have a compelling reason to do otherwise, the MIT license will probably suffice. It’s short, sweet, well-understood, liberal and makes integrating third-party changes back into your own products headache-free. But if you’re contributing to the code that you want to include in a project that already has its own license, you might want to use that license instead. Here’s a useful overview of license types for more info.
  • You’ll want to put a LICENSE file in your repository and have a copyright notice somewhere prominent — either in that file or in the README. Such as ‘(C) 20XX Your Name. All rights reserved.’

Step 6: Name your library or tool

  • Pick a name. Make sure it’s not offensive and avoids having the same name with other existing libraries and trademarked products.

Step 7: Push your code

  • Put it on GitHub, create your own organization, repository and push your code.
  • Keep in mind that some communities have secondary systems that you should consider utilizing as well. If you’re writing .NET, then another one might be Codeplex. If it’s Ubuntu-specific then a Bazaar mirror on Launchpad etc.

Step 8: Publish your package in the appropriate package archive

  • If you’re publishing a library, submit it to the appropriate package manager. For .NET, that would be NuGet; for Python, it’s PyPI; for Perl, it’s CPAN; for Ruby, it’s RubyGems; for Node, it’s NPM; and so on. Also, make sure that someone else at your company, such as a sysadmin, has the ability to continue maintaining the library under the unfortunate circumstance that you get hit by a bus.

Step 9: Announce your code

  • You’re all good, time to announce it! You’ll want to blog and tweet it out. You should also consider publishing on /programming on Reddit and Hacker News etc.

And that’s it! You’re all done!

…well, nearly.

Step 10: Don’t forget about your code

  • Just because you’ve published it doesn’t mean you’re done. You’ve unleashed a new-born into the world; you need to take care of it. Monitor pull requests and bug reports on your new project. If you realize that keeping your project going is overwhelming, then a hearty congratulations! You should remember that it is your responsibility to at least find an extra or substitute maintainer. It’s okay if your project ultimately forks but it’s best not to do so just because you dropped the ball incorporating freely and submitted improvements to your code.

That’s it. For real this time. So go out, contribute, and have fun!

 

Stop More Bugs With This Code Review Checklist!

Checklists are a great tool in code reviews — they ensure that reviews are consistently performed throughout your team. They’re also a handy way to ensure that common issues are identified and resolved.

Research by the Software Engineering Institute suggests that programmers make 15–20 common mistakes. So by adding such mistakes to a checklist, you can make sure that you spot them whenever they occur and help drive them out over time.

To get you started with a checklist, here’s a list of typical items:

Code Review Checklist

General

  • Does the code work? Does it perform its intended function, the logic is correct etc.
  • Is all the code easily understood?
  • Does it conform to your agreed coding conventions? These will usually cover the location of braces, variable and function names, line length, indentations, formatting, and comments.
  • Is there any redundant or duplicate code?
  • Is the code as modular as possible?
  • Can any global variables be replaced?
  • Is there any commented out code?
  • Do loops have a set length and correct termination conditions?
  • Can any of the code be replaced with library functions?
  • Can any logging or debugging code be removed?

Security

  • Are all data inputs checked (for the correct type, length, format, and range) and encoded?
  • Where third-party utilities are used, are returning errors being caught?
  • Are output values checked and encoded?
  • Are invalid parameter values handled?

Documentation

  • Do comments exist and describe the intent of the code?
  • Are all functions commented?
  • Is any unusual behavior or edge-case handling described?
  • Is the use and function of third-party libraries documented?
  • Are data structures and units of measurement explained?
  • Is there any incomplete code? If so, should it be removed or flagged with a suitable marker like ‘TODO’?

Testing

  • Is the code testable? i.e. don’t add too many or hide dependencies, unable to initialize objects, test frameworks can use methods etc.
  • Do tests exist and are they comprehensive? i.e. has at least your agreed on code coverage.
  • Do unit tests actually test that the code is performing the intended functionality?
  • Are arrays checked for ‘out-of-bound’ errors?
  • Could any test code be replaced with the use of an existing API?

You’ll also want to add to this checklist any language-specific issues that can cause problems.

The checklist is deliberately not exhaustive of all issues that can arise. You don’t want a checklist which is so long no-one ever uses it. It’s better to just cover the common issues.

Optimize Your Checklist

Using the checklist as a starting point, you should optimize it for your specific use-case. A great way to do this is to get your team to note the issues that arise during code reviews for a short period of time. With this data, you’ll be able to identify your team’s common mistakes, which you can then build into a custom checklist. Make sure to remove any items that don’t come up (you may wish to keep rarely occurring, yet critical items such as security-related issues).

Get Buy-in and Keep It Up To Date

As a general rule, any items on the checklist should be specific and if possible, something you can make a binary decision about. This helps to avoid inconsistency in judgments. It is also a good idea to share the list with your team and get their approval on the content. Make sure to review the checklist periodically too, to check that each item is still relevant.

Armed with a great checklist, you can raise the number of defects you detect during code reviews. This will help you to drive up coding standards and avoid inconsistent code review quality.