Key point: Keep public header files clean. Clean header files are easier to understand, improve build times, and simplify unit testing.
Matthew Eshleman
Background
The MVP (Minimally Viable Product) is shipping. A stressed development team takes a deep breath while the business team sets terms for another round of funding. Real products with real revenues drive the negotiations. A company is born.
But at what price? Do we dare review the MVP’s code? What confidence does this team have that they can efficiently and regularly add new software features to the product-in-question? The team takes stock and realizes a critical oversight: a lack of unit testing.
To address this oversight, the team tasks Lucy with establishing unit tests. Lucy targets the first selected unit: the alarm module. However, after creating unit tests for the alarm module, the build emits an error:
fatal error: stm32_uart.h: No such file or directory
Strange, how did that header get pulled in? The alarm module (not shown) does not directly use that header. However, alarm.c does require algorithm.h. The build error briefly confuses Lucy, as the unit test build already has a mock equivalent for the algorithm module. Lets take a look at algorithm.h.
The Code
#ifndef ALGORITHM_H
#define ALGORITHM_H
#include <stdint.h>
#include <stddef.h>
#include "log.h"
typedef struct AlgoBuf
{
uint64_t timestamp;
int16_t samples[16];
} AlgorithmBuffer;
typedef enum AlgoState {
ALGO_UNKNOWN,
ALGO_NEED_MORE_DATA,
ALGO_GOOD,
ALGO_MARGINAL,
ALGO_BAD_NEWS_CALL_INSURANCE_CARRIER
} AlgorithmState;
//Init and prepare for incoming data
void AlgorithmInit(void);
//Process new samples and returns updated current state after processing
AlgorithmState AlgorithmProcessData(uint64_t timestamp, int16_t* samples, size_t sampleCount);
//Get last known state
AlgorithmState AlgorithmGetState();
#endif //ALGORITHM_H
The Review
Upon my initial review, algorithm.h is mostly clean. And that is the purpose of this post: ensure that all modules’ public header files are clean. However in this example, we do find a few dirty points:
- The header includes log.h, yet does not use any log related data structures or functions within this header file. My review would note that log.h should be moved into the internal ‘C’ file for the algorithm module, assuming it is actually required by the algorithm module.
- The header defines the data structure AlgorithmBuffer, but does not use it within this header file. It should be removed or moved into a separate private module header file or simply moved into the module’s internal C file.
- There might be other desired changes here, but they are not the focus of this post.
Reasoning and Results
Before we proceed with those changes, why do we care? A few reasons:
- As the number of files included in a build increase, keeping public header files clean has a direct positive impact on build times. The majority of firmware projects fall outside this concern. However, I have supervised projects with 1+ hour build times where even minimal efforts to clean header files resulted in significant improvements in build times.
- “Dirty” public header files increase the efforts required to unit test a module. A dirty header file requires unit test builds to pull in more “cruft.” It may not prevent unit testing, but it will likely bloat any isolated unit test builds or add to the initial effort to begin unit testing a module.
- Clean header files are easier for humans to understand. Keeping the header file clean not only helps the compiler when parsing and building the file, but a clean header file also helps us humans quickly assess and understand a module’s public API and data structures.
- It may be impossible to mock a module with a “dirty” public header file, especially if the unnecessary includes have direct hardware ties.
Lucy spots the above issues and proceeds to move log.h out of the algorithm.h public header, which immediately fixes the compile error. Since the unit test build already has a mock for the algorithm module, the unit tests build and execute. A test fails, but Lucy moves forward with the next step in the team’s unit testing and technical debt drawdown efforts.
Key Point
Reiterating the key point:
Clean header files are easier to understand, improve build times, and simplify unit testing. Keep public header files clean.
Matthew Eshleman
Tip: if you are using a modern IDE, the IDE’s static analysis tools flag unused included header files. Enable that feature and clean up those squiggly lines!
Photo by Towfiqu barbhuiya on Unsplash