Step 3: Eliminate Smelly Code
#############################

..  include::   /header.inc

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:

..  code-block:: bash

    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:

..  code-block:: bash

    $ 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:

..  code-block:: bash

    $ 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:

..  literalinclude::    code/cpu-factory3/Makefile
    :linenos:
    :caption:   Makefile

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.

..  literalinclude::    code/cpu-factory3/CPPLINT.cfg
    :linenos:
    :caption:   CPPLINT.cfg

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

..  code-block:: text

    $ 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:

..  code-block:: c++

    // 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:

..  code-block:: c++

    #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:

..  code-block:: c++

    #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:

..  code-block:: text

    $ 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!