C++ class design with invariant

Question

I've been pondering a really basic question about how far to take enforcing a class's invariant. Maybe that's worded badly, so as an example, let's say that I want to write a class which stores a limited palette of colours. The class's constructor should take the size of the palette - the idea being that you can add as many colours to the palette as you want, but it restricts the size on a FIFO policy. So we'll say that I want to arbitrarily restrict this size to the range 1 to 32 ("nobody will ever need more than 32 colours").

Let's get started...

class palette
{
public:
   palette(unsigned int max_size) : m_max_size{max_size} { }
private:
   unsigned int m_max_size;
};

So far, so good. Except I'm not doing anything to confirm that the max size is within my restricted range, i.e. the invariant can be broken right from construction.

(Just to clarify: this class isn't intended for use in some public library - it's simply an internal class to a single application)

Four options spring to mind beyond "do nothing":

1. Put a comment on the class to ask callers to use the class correctly.

   // Please only call with 1 <= max_size <= 32
   

2. Assert that it's within range.

   assert(max_size >= 1 && max_size <= 32);
   

3. Throw an exception if it's outside the range

   if (max_size < 1 || max_size > 32)
       throw std::invalid_argument();
   

4. Get the compiler to check by converting to a template

   template <unsigned int MAX>
   class palette
   {
   public:
       palette() { // no need for the argument any more
           static_assert(MAX >= 1 && MAX <= 32, "oops");
       }
   };
   

Options #1 seems a little bit like wishful thinking, but maybe it's good enough for 'just' an internal class? Options #2 to #4 are increasingly fussy about the size, to the point where #4 changes the syntax so that the compiler will report an error if the class isn't being used correctly.

I'd welcome any thoughts on this. I understand that the answer will probably begin with "Well it all depends...", but I'm just fishing for some general guidelines or some alternate suggestions.

Answer 1

I would go with an assertion. Without any further information, I'd probably default to the non-static assert().

A comment would indeed be wishful thinking. Not only can it be ignored, but it can very easily go out of date if (when?) you decide 32 should no longer be the limit.

If this was a public API with other programmers using it, the exception would be beneficial as it tends to provide more debugging information, especially if you included a meaningful error message such as "walderFrey::palette size must be between 1 and 32 inclusive". Otherwise, they might have to dig into your source code and learn how your library works just to figure out why that random assert failed. But since this is just for your own use, there's not as much benefit in that. And you might as well get the (admittedly small) performance benefit of having the assert() disappear in non-debug builds.

I've also heard it argued that exceptions should indicate unpredictable, exceptional conditions that may be unlikely to repeat or may not even be fixable, while assertions should indicate programmer errors that definitely can and should be fixed. I generally agree with this guideline when there is no public API to worry about.

The template solution is neat, but that raises a bigger question: should a size-1 palette and a size-32 palette be considered the same type? Does it make sense to assign one to the other? If it does not make sense, then that's an additional benefit of implementing this class as a template. But you should not decide whether to make this class a template solely because a static_assert() is slightly nicer than an assert(). A regular assert() is definitely "good enough". Normally, generic code involving templates is more complex, so I'd default to a "normal" class until I felt the complexity was justified, hence I'd recommend assert() unless you feel going with templates is justified for other reasons.

Answer 2

Use the strongest guarantee that you are willing to maintain for the foreseeable future.

Method 4 (template / static assert) assures that the invariant is always true in a compiled program. However, this implies that the input must be known at compile time, which is not always possible.

Method 2 (assertion) and 3 (exception) can fail at run time, so they offer a weaker guarantee than method 4. The choice between an assertion and an exception depends on the audience (who will use the class), the applicable coding standard and the possible repercussions of failing to maintain the invariant. For an in-depth treatment, see John Lakos - Defensive Programming Done Right.

Use an assertion when you consider this to be a logic (or programmer) error, breaking the invariant is unlikely to cause damage (e.g. damaging hardware, corrupting data) and there is nothing that can be done to prevent the error without changing/recompiling the code. Assertions are typically only checked in a debug build, not in release builds.

Throw an exception if the error can be caused by 'environmental circumstances' (e.g. network failure) or, in some cases, invalid input or if continuing despite the error may cause damage. Of course, the exception should be handled somewhere in the calling code, which essentially means that by throwing an exception you make the semantics of your class more complex.

Method 1 (comment) is code documentation and this should always be done in addition to one of the other methods. You don't want to force someone using this class to have to look at the source code to figure out how to use it. That should be clear from the documentation. If you handle the error by throwing an exception, you should document that too.