Member functions that are sometimes const

Question

I have a class design similar to the following:

class MyClass {
public:
    bool IsValid() const;
    void MakeValid();
private:
    bool CheckValidity(bool fix);
};

bool MyClass::IsValid() const {
    // Check validity, but don't fix any problems found.  Doesn't work.
    return CheckValidity(false);
}

void MyClass::MakeValid() {
    // Check validity and fix problems found.
    CheckValidity(true);
}

IsValid should be const, because it doesn't make changes. MakeValid should be non-const, because it does make changes. They share the same implementation, CheckValidity, but because CheckValidity may or may not make changes, it can't be marked const.

What's the best way to handle this? The simplest approach is to just use const_cast, but casting away const feels a bit dirty:

bool MyClass::IsValid() const {
    // Check validity, but don't fix any problems found.
    return const_cast<MyClass*>(this)->CheckValidity(false);
}

Is this a legitimate use of const_cast? Is there a better approach?

Answer 1

I'm assuming your implementation looks similar to this:

bool CheckValidity(bool fix)
{
    // Actually check validity.
    bool isValid = ...;

    if (!isValid && fix)
    {
        // Attempt to fix validity (and update isValid).
        isValid = ...;
    }

    return isValid;
}

You really have two different functions shoved into one. One of the key indicators of this kind of entanglement is the boolean argument to the function... which smells because the caller cannot immediately discern whether to put true or false without referencing code/docs.

Split up the method:

bool CheckValidity() const
{
    // Actually check validity.
    bool isValid = ...;
    return isValid;
}

void FixValidity()
{
    // Attempt to fix validity.
    // ...
}

And then your public methods can make the calls more appropriately.

bool IsValid() const
{
    // No problem: const method calling const method
    return CheckValidity();
}

void MakeValid()
{
    if (!CheckValidity())  // No problem: non-const calling const
    {
         FixValidity();    // No problem: non-const calling non-const
    }
}