15

Are there any established patterns for checking class invariants in C++?

Ideally, the invariants would be automatically checked at the beginning and at the end of each public member function. As far as I know, C with classes provided special before and after member functions, but unfortunately, design by contract wasn't quite popular at the time and nobody except Bjarne used that feature, so he removed it.

Of course, manually inserting check_invariants() calls at the beginning and at the end of each public member function is tedious and error-prone. Since RAII is the weapon of choice to deal with exceptions, I came up with the following scheme of defining an invariance checker as the first local variable, and that invariance checker checks the invariants both at construction and destruction time:

template <typename T>
class invariants_checker
{
    const T* p;

public:

    invariants_checker(const T* p) : p(p)
    {
        p->check_invariants();
    }

    ~invariants_checker()
    {
        p->check_invariants();
    }
};

void Foo::bar()
{
    // class invariants checked by construction of _
    invariants_checker<Foo> _(this);

    // ... mutate the object

    // class invariants checked by destruction of _
}

Question #0: I suppose there is no way to declare an unnamed local variable? :)

We would still have to call check_invariants() manually at the end of the Foo constructor and at the beginning of the Foo destructor. However, many constructor bodies and destructor bodies are empty. In that case, could we use an invariants_checker as the last member?

#include <string>
#include <stdexcept>

class Foo
{
    std::string str;
    std::string::size_type cached_length;
    invariants_checker<Foo> _;

public:

    Foo(const std::string& str)
    : str(str), cached_length(str.length()), _(this) {}

    void check_invariants() const
    {
        if (str.length() != cached_length)
            throw std::logic_error("wrong cached length");
    }

    // ...
};

Question #1: Is it valid to pass this to the invariants_checker constructor which immediately calls check_invariants via that pointer, even though the Foo object is still under construction?

Question #2: Do you see any other problems with this approach? Can you improve it?

Question #3: Is this approach new or well-known? Are there better solutions available?

fredoverflow
  • 237,063
  • 85
  • 359
  • 638
  • You cannot use `this` in initializer list. You could use it however in the body of the constructor. – Benoit Jan 19 '11 at 14:01
  • @Benoit: What do you mean with *cannot*? Is it strictly forbidden? Does it invoke undefined behavior? – fredoverflow Jan 19 '11 at 14:04
  • 1
    Thorsten Ottesen (I think it was) had a proposal for Design By Contract. It didn't get off the ground in the first round, due to difficulty of deciding what's internal and what's external call (you can temporarily break invariant for internal call). But it might still show up. Don't know if it's actively worked on. – Cheers and hth. - Alf Jan 19 '11 at 14:12
  • @Question1: Its safe, but check_variants should not be virtual.. – smerlin Jan 19 '11 at 14:15
  • 2
    @smerlin: in fact everything is a little trickier than that. On the one side, you can pass pointers to the yet uninitialized object around (that's `this` here), but you cannot dereference it until the initialization list has completed the execution. On the other hand, there is no particular problem in calling a virtual function from the constructor, it will call the `base` overrider and not the *final overrider*, but that will not in itself be a problem, unless it is a pure virtual method in which case it will call `terminate()`. – David Rodríguez - dribeas Jan 19 '11 at 14:39
  • @Alf: Sounds interesting. Do you have a link for that proposal? – fredoverflow Jan 19 '11 at 15:12
  • @David: Hmm i missread the Question, i thought he just wants to call the check_invariants function in the constructor, my bad... And of course you are right that calling a virtual function in the constructor wont cause harm normally, but because many people dont know the actual behaviour of calls to virtual functions in constructors and destructors it should be avoided whenever possible. – smerlin Jan 19 '11 at 15:34
  • @FredOverflow: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n1962.html Huh, I'm mentioned in the acknowledgments. No wonder I remembered it. :-) – Cheers and hth. - Alf Jan 19 '11 at 20:46

6 Answers6

11

Answer #0: You can have unnamed local variables, but you give up control over the life time of the object - and the whole point of the object is because you have a good idea when it goes out of scope. You can use

void Foo::bar()
{
    invariants_checker<Foo>(this); // goes out of scope at the semicolon
    new invariants_checker<Foo>(this); // the constructed object is never destructed
    // ...
}

but neither is what you want.

Answer #1: No, I believe it's not valid. The object referenced by this is only fully constructed (and thus starts to exist) when the constructor finished. You're playing a dangerous game here.

Answer #2 & #3: This approach is not new, a simple google query for e.g. "check invariants C++ template" will yield a lot of hits on this topic. In particular, this solution can be improved further if you don't mind overloading the -> operator, like this:

template <typename T>
class invariants_checker {
public:
  class ProxyObject {
  public:
    ProxyObject(T* x) : m(x) { m->check_invariants(); }
    ~ProxyObject() { m->check_invariants(); }
    T* operator->() { return m; }
    const T* operator->() const { return m; }
  private:
    T* m;
  };

invariants_checker(T* x) : m(x) { }

ProxyObject operator->() { return m; } 
const ProxyObject operator->() const { return m; }

private:
   T* m;
};

The idea is that for the duration of a member function call, you create an anonymous proxy object which performs the check in its constructor and destructor. You can use the above template like this:

void f() {
  Foo f;
  invariants_checker<Foo> g( &f );
  g->bar(); // this constructs and destructs the ProxyObject, which does the checking
}
Frerich Raabe
  • 81,733
  • 18
  • 105
  • 196
  • 1
    0) is not an unnamed variable, but rather a temporary object. There is no variable declared but a single statement that creates a temporary and does nothing with it. +1 for the last block and the suggestion of overloading `operator->`... fancy little beast there. (Another similar use is shown in `Modern C++ design` to provide a locking smart pointer. Suggestions to add a locking smart pointer to the standard were rejected on the grounds that the granularity of the locks would in most cases not be the best choice) – David Rodríguez - dribeas Jan 19 '11 at 14:44
  • 1
    @David: Then what's the difference between an unnamed variable (AKA an anonymous value) and a temporary object? – Frerich Raabe Jan 19 '11 at 15:41
  • 1
    @David, That doesn't create "unnamed variables". Variables are created by declarations. Not by `new` or by functional casts... 12.1/13 says "unnamed object", not "unnamed variable". unnamed class objects don't need to be temporaries. Consider `void f(string) { }`. The parameter is unnamed, but is not a temporary. – Johannes Schaub - litb Jan 19 '11 at 16:31
  • @litb: You are right (as always :)) I have deleted the comments, this should answer @Frerich Raabe's comment above. Thanks. – David Rodríguez - dribeas Jan 19 '11 at 16:52
  • 1
    @Frerich: the proxy idea is nice, however you gave up the control of your invariants to the caller. – Matthieu M. Jan 19 '11 at 18:10
  • @MatthieuM You could pass a functor to the constructor of `invariants_checker` which performs the invariants checking, and the ctor/dtor of the template just calls that functor. – Frerich Raabe Mar 26 '14 at 08:20
2

Ideally, the invariants would be automatically checked at the beginning and at the end of each public member function

I think this is overkill; I instead check invariants judiciously. The data members of your class are private (right?), so only its member functions can change the data memebers and therefore invalidate invariants. So you can get away with checking an invariant just after a change to a data member that particiaptes in that invariant.

Raedwald
  • 40,290
  • 35
  • 127
  • 207
1

Question #0: I suppose there is no way to declare an unnamed local variable? :)

You can usually whip up something using macros and __LINE__, but if you just pick a strange enough name, it should already do, since you shouldn't have more than one (directly) in the same scope. This

class invariants_checker {};

template<class T>
class invariants_checker_impl : public invariants_checker {
public:
    invariants_checker_impl(T* that) : that_(that) {that_->check_invariants();}
    ~invariants_checker_impl()                     {that_->check_invariants();}
private:
    T* that_;
};

template<class T>
inline invariants_checker_impl<T> get_invariant_checker(T* that)
{return invariants_checker_impl<T>(that);}

#define CHECK_INVARIANTS const invariants_checker& 
   my_fancy_invariants_checker_object_ = get_invariant_checker(this)

works for me.

Question #1: Is it valid to pass this to the invariants_checker constructor which immediately calls check_invariants via that pointer, even though the Foo object is still under construction?

I'm not sure whether it invokes UB technical. In practice it would certainly be safe to do so - where it not for the fact that, in practice, a class member that has to be declared at a specific position in relation to other class members is going to be a problem sooner or later.

Question #2: Do you see any other problems with this approach? Can you improve it?

See #2. Take a moderately sized class, add half a decade of extending and bug-fixing by two dozen developers, and I consider the chances to mess this up at at least once at about 98%.
You can somewhat mitigate this by adding a shouting comment to the data member. Still.

Question #3: Is this approach new or well-known? Are there better solutions available?

I hadn't seen this approach, but given your description of before() and after() I immediately thought of the same solution.

I think Stroustrup had an article many (~15?) years ago, where he described a handle class overloading operator->() to return a proxy. This could then, in its ctor and dtor, perform before- and after-actions while being oblivious to the methods being invoked through it.

Edit: I see that Frerich has added an answer fleshing this out. Of course, unless your class already needs to be used through such a handle, this is a burden onto your class' users. (IOW: It won't work.)

Community
  • 1
  • 1
sbi
  • 204,536
  • 44
  • 236
  • 426
1

#0: No, but things could be slightly better with a macro (if you're ok with that)

#1: No, but it depends. You cannot do anything that would cause this to be dereferenced in before the body (which yours would, but just before, so it could work). This means that you can store this, but not access fields or virtual functions. Calling check_invariants() is not ok if it's virtual. I think it would work for most implementations, but not guaranteed to work.

#2: I think it will be tedious, and not worth it. This have been my experience with invariant checking. I prefer unit tests.

#3: I've seen it. It seems like the right way to me if you're going to do it.

Lou Franco
  • 83,503
  • 14
  • 127
  • 183
0

unit testing is better alternative that leads to smaller code with better performance

Andriy Tylychko
  • 15,244
  • 5
  • 56
  • 103
0

I clearly see the issue that your destructor is calling a function that will often throw, that's a no-no in C++ isn't it?

CashCow
  • 29,087
  • 4
  • 53
  • 86
  • Absolutely -- because destructors are called while unwinding the stack during an exception. There is no way to know the difference. However, I expect invariant checking to only be there during testing, not production code -- so it might be ok. – Lou Franco Jan 19 '11 at 15:04
  • 1
    @Lou: If the invariant check fires, that means there's a programming error in the code and the class is in a state it should never be in. In this case all bets are off, and the program should simply terminate. Whether this is discovered in a dtor __doesn't matter__ at all, when the program is going to terminate anyway – sbi Jan 19 '11 at 16:13