By Dan Bader / December 22, 2016

Don’t Make This Code Review Mistake

Ever introduced code reviews to an existing code base?

It can be awesome, or pure hell –

On that nice Vancouver morning I sat down with a fresh cup of coffee, ready to dig in and give some feedback on a fix we wanted to ship before the end of the sprint.

When I loaded up the first set of changes into my trusty Sublime Text, my eyes nearly fell out 👀

This was a serious “can’t see the forest for the trees” type of situation:

The formatting for this code was All. Over. The. Place.

There was no consistency whatsoever in how the code was indented, how braces were positioned… even the spacing between operators inside expressions was seemingly randomized: value +=10*  othervalue

Ugh.

It just seemed so sloppy! And the inconsistent formatting made it really hard to see what the code did, what the intention behind it was.

It felt like my brain was 90% occupied with parsing out the code, instead of being able to focus on the bigger picture and to hunt for actual bugs.

I must’ve spent at least an hour cleaning up the formatting, before I was able to give any substantial feedback on these changes. It was the most tedious code review of my dev career.

My busywork was of little value to the company, too.

They paid me a software engineer’s salary for nudging around braces and juggling whitespace…

That same day I pulled the whole team together to discuss the mandatory use of a code style checker before code reviews.

And guess what? It worked out great.

Most developers on the team were using Sublime Text so we all installed the SublimeLinter package. It’s the most popular code linting framework for Sublime Text and I like it for its focus, simplicity, and performance.

A code linter is a program that analyses your source code for potential errors. Code linters are great at finding “mechanical” issues like syntax errors, structural problems, such as the use of undefined variables, and also best practice or code style violations.

SublimeLinter let’s you integrate code linting feedback into your editing environment.

Setting up SublimeLinter gives you immediate feedback on your code right when you type it:

When you install SublimeLinter it doesn’t actually include any linter engines. It’s more like a “meta linter” that lets you integrate various command-line linter binaries like Flake8 (Python) or JSHint (JavaScript) under one roof.

The linter binaries do the real work. And that way, SublimeLinter can support more than just one programming language. If you’re doing any kind of full-stack web development, for example, you could install code linters for JavaScript, CSS, Ruby, Go, and Python.

SublimeLinter will then pick the right code linter to run on each file you’re editing. Any errors or warnings found by these separate linters would all be integrated with the same look and feel into your Sublime Text editor window by SublimeLinter.

And because we were using command line tools through SublimeLinter we were able to set up the same set of code style checks on our CI build server very easily. That way no badly formatted code could slip through the cracks ever again.

It made the whole team more productive. And it was great for morale: No more time wasted on nudging braces or juggling whitespace 🙂

Additional Resources & Links

Here are a couple of extra links to help you get set up with SublimeLinter. I listed the most common linter binaries and linter plugins so you can get started right away:

About the author

Dan Bader

Hey, I’m Dan and I love helping developers take their coding skills and productivity to the next level. I’m an independent software engineer, productivity nut, author, and speaker. I’ve been developing software professionally for 15 years and I want to help you get more awesome at your job, too!

2 comments
Lutz - December 22, 2016

Have you ever seen legacy code older then 10 years?
Started in the good old times with python 1.5x?

I had a problem this week with this type of code. The Interpreter was confused an the traceback was not very helpful. The message has nothing todo with the real problem: mixed indention some where in a couple of lines in the more of thousand lines before (one method!!!).
After fixing the indention we are starting to debug the real problem. Just wasted more then three hours.

I haver no idea how to refactor this code because we don’t have automatic tests for it.

Reply
Martin - December 22, 2016

I have to deal with this every day… see, problem is that my co-workers knowledge is limited to internet tutorials… I don’t even know why my boss keeps hiring this kind of under-trained ppl.. I guess it’s cheap salaries they must be paying to them. Zero linux, git, code-standards or security knowledge.. Their code is sometimes so bad I have to rewrite entire scripts because it will take less time that optimize, secure and fix the mess they wrote. Sucks to be me 😛

Reply
Click here to add a comment

Leave a comment: