c# - How do you validate an object's internal state?

Question

I'm interested in hearing what technique(s) you're using to validate the internal state of an object during an operation that, from it's own point of view, only can fail because of bad internal state or invariant breach.

My primary focus is on C++, since in C# the official and prevalent way is to throw an exception, and in C++ there's not just one single way to do this (ok, not really in C# either, I know that).

Note that I'm not talking about function parameter validation, but more like class invariant integrity checks.

For instance, let's say we want a Printer object to Queue a print job asynchronously. To the user of Printer, that operation can only succeed, because an asynchronous queue result with arrive at another time. So, there's no relevant error code to convey to the caller.

But to the Printer object, this operation can fail if the internal state is bad, i.e., the class invariant is broken, which basically means: a bug. This condition is not necessarily of any interest to the user of the Printer object.

Personally, I tend to mix three styles of internal state validation and I can't really decide which one's the best, if any, only which one is absolutely the worst. I'd like to hear your views on these and also that you share any of your own experiences and thoughts on this matter.

The first style I use - better fail in a controllable way than corrupt data:

void Printer::Queue(const PrintJob& job)
{
    // Validate the state in both release and debug builds.
    // Never proceed with the queuing in a bad state.
    if(!IsValidState())
    {
        throw InvalidOperationException();
    }

    // Continue with queuing, parameter checking, etc.
    // Internal state is guaranteed to be good.
}

The second style I use - better crash uncontrollable than corrupt data:

void Printer::Queue(const PrintJob& job)
{
    // Validate the state in debug builds only.
    // Break into the debugger in debug builds.
    // Always proceed with the queuing, also in a bad state.
    DebugAssert(IsValidState());

    // Continue with queuing, parameter checking, etc.
    // Generally, behavior is now undefined, because of bad internal state.
    // But, specifically, this often means an access violation when
    // a NULL pointer is dereferenced, or something similar, and that crash will
    // generate a dump file that can be used to find the error cause during
    // testing before shipping the product.
}

The third style I use - better silently and defensively bail out than corrupt data:

void Printer::Queue(const PrintJob& job)
{
    // Validate the state in both release and debug builds.
    // Break into the debugger in debug builds.
    // Never proceed with the queuing in a bad state.
    // This object will likely never again succeed in queuing anything.
    if(!IsValidState())
    {
        DebugBreak();
        return;
    }

    // Continue with defenestration.
    // Internal state is guaranteed to be good.
}

My comments to the styles:

1. I think I prefer the second style, where the failure isn't hidden, provided that an access violation actually causes a crash.
2. If it's not a NULL pointer involved in the invariant, then I tend to lean towards the first style.
3. I really dislike the third style, since it will hide lots of bugs, but I know people that prefers it in production code, because it creates the illusion of a robust software that doesn't crash (features will just stop to function, as in the queuing on the broken Printer object).

Do you prefer any of these or do you have other ways of achieving this?

Answer 1

You can use a technique called NVI (Non-Virtual-Interface) together with the template method pattern. This probably is how i would do it (of course, it's only my personal opinion, which is indeed debatable):

class Printer {
public:
    // checks invariant, and calls the actual queuing
    void Queue(const PrintJob&);
private:
    virtual void DoQueue(const PringJob&);
};


void Printer::Queue(const PrintJob& job) // not virtual
{
    // Validate the state in both release and debug builds.
    // Never proceed with the queuing in a bad state.
    if(!IsValidState()) {
        throw std::logic_error("Printer not ready");
    }

    // call virtual method DoQueue which does the job
    DoQueue(job);
}

void Printer::DoQueue(const PrintJob& job) // virtual
{
    // Do the actual Queuing. State is guaranteed to be valid.
}

Because Queue is non-virtual, the invariant is still checked if a derived class overrides DoQueue for special handling.

---

To your options: I think it depends on the condition you want to check.

If it is an internal invariant

If it is an invariant, it should not be possible for a user of your class to violate it. The class should care about its invariant itself. Therefor, i would assert(CheckInvariant()); in such a case.

It's merely a pre-condition of a method

If it's merely a pre-condition that the user of the class would have to guarantee (say, only printing after the printer is ready), i would throw std::logic_error as shown above.

I would really discourage from check a condition, but then doing nothing.

---

The user of the class could itself assert before calling a method that the pre-conditions of it are satisfied. So generally, if a class is responsible for some state, and it finds a state to be invalid, it should assert. If the class finds a condition to be violated that doesn't fall in its responsibility, it should throw.