Code Design Guidelines
Here is a list of code "smells" and heuristics adapted from several sources. Like the style guide, these are subjective rules, however they represent the consensus of many experienced programmers, and we may deduct some points for violating them.
Single Responsibility Principle
Any block of code, Module, class, function, or method should do one well-defined thing, and do it well.
- Do not use comments to replace information that can be encoded in the language. For example, that a method is constant. Remember comments have no effect on the compiler.
- Do not include information that can be generated from other places, such as version history, change logs, etc.
- Do not leave obsolete comments around
- Don't over comment, for example redundant type descriptions in function definitions.
- Keep comments brief
- Don't use comments to keep old code you want to save, use proper version control
Builds and tests should take a single step
You should be get the code, configure, and built it in at most three commands that are ideally collecting into a single script. Example:
$ git clone mycode
$ cmake -H mycode -B_build -DCMAKE_BUILD_TYPE=Debug
$ cmake --build _build --config Debug
After building you should be able to run tests as a single step. Example:
$ cd _build; ctest
Functions and Methods
- Function names should say what they do in a specific sense.
- Keep the number of function arguments under three. More than that indicates the code needs to be refactored.
- Functions should be short and do one thing. No function should be larger than you can see on the screen at once and preferably shorter.
- Break calculations up into meaningful intermediate expressions and name the variables accordingly. The compiler is good at optimizing this out and it enhances readability.
- When possible do not use output arguments, those passed by reference or pointer should be marked const.
- Boolean flags in arguments indicate a violation of the single responsibility principle. Create a separate function instead.
- Remove functions not called (dead functions).
Classes
- Name classes by what they are or what they do. For example, not just
Writer
or even FileWriter
, but ConfigurationFileWriter
.
- The public section of a class should be as small as possible. Obese classes indicate a violation of the single responsibility principle.
- Avoid writing accessors (getters and setters) for every private member. If you do it should enforce a constraint.
Tests
- Test everything. Ideally every line of code is executed by a test, even things that look trivial.
- Test boundary conditions and any code that generates errors or exceptions. For example, what happens if an integer wraps?
- Tests should be fast, or split into fast and slower sets.
See also Unit Testing with Catch and CMake.
Misc
- Do not use magic numbers. Define static constants in the proper scope (namespace, implementation file, or class).
- Do not write complicated conditionals. They are difficult to reason about and can hide bugs. Instead encapsulate the logic in a function that can be tested.
- The length and descriptiveness of variable names should increase with scope. For example, it is ok to name a variable local to a loop with a one letter name, but not a class member.