Step 3: Eliminate Smelly Code

Read time: 13 minutes (3301 words)

Does your code smell? Not literally, of course. But when someone other than you tries to read your code, does their nose wrinke up because they are having a tough time even reading your code, much less understanding it?

Good Style in programming is a serious business. When you join a real development team, you will discover that they have rules that dictate how you write your code. The rules are followed by everyone on the team. The goal is to reduce the “smell” of project code.

Some companies take this so seriously that they build tools to check your code and verify that you are following the rules.

Google is one of those companies! The tool they built is actually a Python tool named cpplint. We will use this in our class projects to make sure you are writing clean code that does not smell! (Hey, Google invented this tool, I like it, except that I had to unlearn a few techniques I used to use in my code!)

Installing cpplint

Since this is a Python tool, we need to make sure we have Python installed. In this case, we will use Python3, so here is the check:

python3 --version
Python 3.6.5

(That is my Macbook version, yours might be newer!)

We also need a tool that is quite common in the Python development world: pip.

Let’s check that one:

$ pip3 --version
pip 9.0.3 from /Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages (python 3.6)

With pip installed, all we need to do to install ``cpplint is this:

$ pip3 install cpplint

With this package installed, we need to modify our Makefile so we can check our code.

Here is the modification, just a new target at the bottom of the file:

Makefile

TARGET = demo
SSRCS	= $(wildcard src/*.cpp)
LSRCS	= $(wildcard lib/*.cpp)
OBJS	= $(SSRCS:.cpp=.o) $(LSRCS:.cpp=.o)
CFLAGS	= -I include -std=c++11

.PHONY: all
all:	$(TARGET)

$(TARGET):	$(OBJS)
	g++ -o $@ $^

%.o:	%.cpp
	g++ -c $(CFLAGS) -o $@ $<

.PHONY: run
run:	$(TARGET)
	./$(TARGET)

.PHONY: clean
clean:
	rm -f $(TARGET) $(OBJS)

.PHONY:	lint
lint:
	python3 -m cpplint --recursive --quiet include src lib

We need a configuration file that will make cpplint behave a bit better. We do not need to worry about the contents of this file for now.

CPPLINT.cfg

set noparent
root=.
linelength=80
extensions=cpp,h
exclude_files=.*\.hpp
filter=-build/include_subdir,-runtime/threadsafe_fn

Running this new command produces something we should not like - it detects “smelly code” (at least as defined by Google!)

$ make lint
python3 -m cpplint --recursive --quiet include src lib
src/main.cpp:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
src/main.cpp:5:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
Ignoring include/catch.hpp; not a valid file name (cpp, h)
include/version.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
include/version.h:0:  No #ifndef header guard found, suggested CPP variable is: LECTURES_04_CPU_FACTORY_CODE_CPU_FACTORY3_INCLUDE_VERSION_H_  [build/header_guard] [5]
include/version.h:4:  For a static/global string constant, use a C style string instead: "const char version[]".

Let’s deal with these issues, one at a time

Copyright

Google insists that the author of a file claim credit for it. The easy way to do that is to add a copyright line with your name and the date. Here is a line from my file:

// Copyright 2019 Roie R. Black

If more than one person worked on a file, list all of their names.

Place this line at the top of all code files you write.

Extra Whitespace

If you end a line with whitespace, cpplint will complain. This is an easy fix.

Header Guards

It is important to include a header guard in C++ header files, to prevent the accidental reading of a header file twice. This is easy to do if you have a ton of includes in your code. In older C++ code, we used to write something line this:

#Ifndef VERSION_H
#define VERSION_H
...
#endif

This is annoying, since you need to invent a name for your “guard”, and in a big project, that can be difficult. The error message produced by cpplint shows you the ugly way Google deals with guard naming. I think that smells even worse!

Modern C++ developers, at least those using modern compilers, do this instead:

#pragma once

Place this line at the top of your header file, and the problem is gone.

Making the changes needed to silence the make lint command is not that much effort. Your code may even look more pretty as a result.

Warning

Some of the style issues you will run into deal with how you put whitespace in your code. My style has always been about what Google wants, so I have few issues with this. YMMV!

Here is what I see with a little tweaking of my code:

$ make lint
python3 -m cpplint --recursive --quiet include src lib
Ignoring include/catch.hpp; not a valid file name (h, cpp)

i have not figured out how to silence that last line. It is not an error, just a message.

That is enough for this week’s lab project. Next up we will build a real (simple) machine, using a few parts and wires. Stay tuned!