— Onur Mustak Cobanli
In my previous post on this subject, I introduced Scott Meyer’s most important interface design rule: “Make interfaces easy to use correctly and hard to use incorrectly”. As a case in point, we looked at a function which returns a temperature reading from a sensor plus a status code. The temperature sensor might be temporarily or permanently unavailable, so the temperature value can only be relied upon if the returned status is “good”. “Making interfaces hard to use incorrectly” means in this case “making it hard to ignore the returned status”. I claimed that this version of getTemperature is a move in the right direction:
|
1 2 3 |
[[nodiscard]] Status getTemperature(int* temperature); |
Still, it’s not perfect for at least four reasons. First of all, the [[nodiscard]] attribute is only available if your compiler supports the C++17 language standard. Second, like I explained in my previous post, the C++17 language standard only “recommends” that a compiler issues a warning. Third, even if your compiler does issue a warning, a programmer might disable this/some/all warnings for a particular project, module, or function. But even if none of these reasons apply, a programmer might still dodge checking the status value.
The easiest way to suppress a warning resulting from an unchecked [[nodiscard]] return value is by casting the returned value to void:
|
1 2 3 4 5 6 7 |
int temperature; (void) getTemperature(&temperature); if (temperature == 100) { // Water is boiling. } |
You probably think that such developers are stupid and that they are putting their head on the block, that they deserve the pain caused by their deliberate misuse. I certainly agree, but in safety-critical systems, somebody other than the developer might actually suffer the pain, either physically or financially.
In other, more insidious cases, bypassing of [[nodiscard]] can happen accidentally:
|
1 2 3 4 5 6 7 8 9 |
int temperature; Status status = getTemperature(&temperature); logger << "Temperature: " << temperature << ", " << "Status: " << statusToString(status) << "\n"; if (temperature == 100) { // Water is boiling. } |
Here, the return value is accessed (for logging), so no warning is issued by the compiler. Nevertheless, the value is not checked and thus the code is still buggy. What we need is a status code that enforces that its value is checked. Enter class Checked.
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 |
template<class T> class [[nodiscard]] Checked final { public: Checked(T value) : value_(value) {} ~Checked() { if (! checked_) std::terminate(); } Checked(const Checked&) = delete; T& operator=(T value) { value_ = value; return value_; } T value() const { return value_; } private: template<class TT> friend bool operator==(Checked<TT>& e, const TT& t); template<class TT> friend bool operator!=(Checked<TT>& e, const TT& t); T value_; bool checked_ { false }; }; template<class T> bool operator==(Checked<T>& e, const T& t) { e.checked_ = true; return e.value_ == t; } template<class T> bool operator!=(Checked<T>& e, const T& t) { return !(e == t); } |
All but the most trivial template code can look intimidating, but Checked is actually quite simple. It acts as a wrapper around a type (Status, in our case) and stores a value of that type. At construction time, a flag called checked is set to false. If this flag is still false when the object is destructed, std::terminate() will be called. The only way to set the flag to true is by invoking the comparison operators == and !=. Here are some examples:
|
1 2 3 4 5 |
{ Checked<Status> status(Status::bad); } // <-- status not checked, std::terminate called! |
|
1 2 3 4 5 6 7 8 |
{ Checked<Status> status(Status::bad); if (status != Status::good) { ... } } // <-- status checked, everything fine! |
Applied to our getTemperature function, we get
|
1 2 3 4 5 6 7 8 9 10 11 12 13 |
Checked<Status> getTemperature(int* temperature) { ... } ... { int temperature; auto status = getTemperature(&temperature); logger << "Temperature: " << temperature << ", " << "Status: " << statusToString(status.value()) << "\n"; if (temperature == 100) { // Water is boiling. } } // <-- status not checked, std::terminate called! |
Even though the status value is accessed for logging, std::terminate will still be called because we failed to actually check the status value. The correct usage looks like this:
|
1 2 3 4 5 6 7 8 9 10 11 12 |
... { int temperature; Checked<Status> status = getTemperature(&temperature); if (status == Status::good) { if (temperature == 100) { // Water is boiling. } } } // <-- status checked, everything fine! |
Did you notice that I tagged the whole Checked class with the [[nodiscard]] attribute? As a consequence, all your functions that return Checked values are implicitly declared [[nodiscard]]. Less typing, less risk of forgetting to add it. Cool!
By using Checked you explicitly communicate to users of your code that they must check the value. The first line of defense is the [[nodiscard]] check at compile-time. If callers cast the returned value to (void) or otherwise fail to check/compare the returned value, they’ll get busted by std::terminate. The upshot of using Checked is that your interfaces are much harder to use incorrectly than by just using [[nodiscard]]. Let’s hope Scott Meyers is satisfied now.