Sell Me on Const Correctness

Sell me on const correctness

This is the definitive article on "const correctness": https://isocpp.org/wiki/faq/const-correctness.

In a nutshell, using const is good practice because...

1. It protects you from accidentally changing variables that aren't intended be changed,
2. It protects you from making accidental variable assignments, and

3. The compiler can optimize it. For instance, you are protected from

   if( x = y ) // whoops, meant if( x == y )
   

At the same time, the compiler can generate more efficient code because it knows exactly what the state of the variable/function will be at all times. If you are writing tight C++ code, this is good.

You are correct in that it can be difficult to use const-correctness consistently, but the end code is more concise and safer to program with. When you do a lot of C++ development, the benefits of this quickly manifest.

Is it worth to insert `const`-correctness

Should I invest the amount of time to introduce const-correctness into this code?

If you feel that you can get it all done in a reasonable time, sure. const-correctness is a good thing, so if you can adjust the codebase to employ it properly then that can never be bad.

It all comes down to how much time you have available and what else you could be doing instead, which is more about project management and more appropriate on programmers.SE.

Even worse, I have to touch and alter the old mature code and explain to the seniors what I did during the code review. Is it worth it?

It's certainly worth it for them (and, by extension, to everybody else). It sounds like they will learn a lot in code review, which is fantastic!

---

Edit

As molbdnilo rightly points out, this is a big change and you should definitely have a group discussion about it before starting. This would be more appropriate than leaving it to code review in two weeks' time, when you've already done it.

Why is const-correctness specific to C++?

Well, it will have taken me 6 years to really understand, but now I can finally answer my own question.

The reason C++ has "const-correctness" and that Java, C#, etc. don't, is that C++ only supports value types, and these other languages only support or at least default to reference types.

Let's see how C#, a language that defaults to reference types, deals with immutability when value types are involved. Let's say you have a mutable value type, and another type that has a readonly field of that type:

struct Vector {
    public int X { get; private set; }
    public int Y { get; private set; }
    public void Add(int x, int y) {
        X += x;
        Y += y;
    }
}

class Foo {
    readonly Vector _v;
    public void Add(int x, int y) => _v.Add(x, y);
    public override string ToString() => $"{_v.X} {_v.Y}";
}

void Main()
{
    var f = new Foo();
    f.Add(3, 4);
    Console.WriteLine(f);
}

What should this code do?

1. fail to compile
2. print "3, 4"
3. print "0, 0"

The answer is #3. C# tries to honor your "readonly" keyword by invoking the method Add on a throw-away copy of the object. That's weird, yes, but what other options does it have? If it invokes the method on the original Vector, the object will change, violating the "readonly"-ness of the field. If it fails to compile, then readonly value type members are pretty useless, because you can't invoke any methods on them, out of fear they might change the object.

If only we could label which methods are safe to call on readonly instances... Wait, that's exactly what const methods are in C++!

C# doesn't bother with const methods because we don't use value types that much in C#; we just avoid mutable value types (and declare them "evil", see 1, 2).

Also, reference types don't suffer from this problem, because when you mark a reference type variable as readonly, what's readonly is the reference, not the object itself. That's very easy for the compiler to enforce, it can mark any assignment as a compilation error except at initialization. If all you use is reference types and all your fields and variables are readonly, you get immutability everywhere at little syntactic cost. F# works entirely like this. Java avoids the issue by just not supporting user-defined value types.

C++ doesn't have the concept of "reference types", only "value types" (in C#-lingo); some of these value types can be pointers or references, but like value types in C#, none of them own their storage. If C++ treated "const" on its types the way C# treats "readonly" on value types, it would be very confusing as the example above demonstrates, nevermind the nasty interaction with copy constructors.

So C++ doesn't create a throw-away copy, because that would create endless pain. It doesn't forbid you to call any methods on members either, because, well, the language wouldn't be very useful then. But it still wants to have some notion of "readonly" or "const-ness".

C++ attempts to find a middle way by making you label which methods are safe to call on const members, and then it trusts you to have been faithful and accurate in your labeling and calls methods on the original objects directly. This is not perfect - it's verbose, and you're allowed to violate const-ness as much as you please - but it's arguably better than all the other options.

const std::shared_ptr& const correctness

The pointer is constant not the object. You can't change which object the pointer is pointing to but you can change the object.

If you want a constant object you need:

void FN(const std::shared_ptr<const K>& k)
{
    k->Change();
}

See also: https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/ for why you might not want to use a const shared_ptr parameter at all.

How to enforce const-correctness regarding pointer data-members

You might use (or reimplement) std::experimental::propagate_const

and then your code would be:

class Widget {
public:
    void Foo();
    void FooConst() const;
};

class WidgetManager {
public:
    WidgetManager() : _pW(std::make_shared<Widget>()) {}

    void ManagerFoo()
    {
        _pW->Foo();      // OK
        _pW->FooConst(); // OK
    }

    void ManagerFooConst() const
    {
        _pW->Foo();         // not compile
        _pW->FooConst();    // OK
    }

private:
    std::experimental::propagate_const<std::shared_ptr<Widget>> _pW;
};

Demo

const correctness with const objects and member pointers, constructor vulnerability

Clearly the constructor (or at least the initializer list if not the body of the ctor) needs to be able to write a value to i.

As it happens, the way C++ achieves this is to make this a pointer-to-non-const in the constructor (and destructor). Basically, the const-ness of obj doesn't commence until the constructor has finished executing. That's why the vulnerability exists, because of a straightforward but imperfect solution to the technical problem of how to construct const-qualified objects.

Perhaps it could in principle be done differently. I suppose you'd need a separate const version of the constructor in which the compiler applies different rules (just as normal member functions can be const), treating data members as const and therefore (1) allowing them to be initialized but not assigned, (2) forbidding the initialization of ptr from &i since the latter would have type int const*. C++ doesn't do this, and as a result it has this loophole that you've driven through. If it did do it, people would have more difficulty writing constructors in certain cases, so it's a design trade-off.

Be aware that similarly a volatile-qualified object is not volatile in its own constructor or destructor.

Am I breaking const-correctness if I expose a const and non-const API?

It depends on your semantics, but I don't think I would use the functions you have here. Instead, here are the two options I would consider:

If you want A to own this count variable, then you should probably preserve const-ness when accessing it. In this case, you might write your functions like this:

void GetCount(const int** ppCount) const
{
    *ppCount = m_pCount;
}

void GetCount(int** ppCount) // note - omitted const
{
    *ppCount = m_pCount;
}

This means that if you have a non-const A you can get a non-const counter, but if you have a const A you can only get a const counter.

---

You could also mean that A simply observes some count variable. In that context, a const observer means that you're not going to change which int you're pointing to, but that thing itself might change. In that case, you might write your accessor like this:

void GetCount(int** ppCount) const
{
    *ppCount = m_pCount;
}

---

In general I would say to prefer the first method and preserve const-ness, but the other way is certainly valid in certain circumstances.

Const correctness in C++

First. Sice you are returning by value, it doesn't really make any difference if you are returning const range or range.

Second. auto r = array.all(); creates a copy of whatever was returned by all. Your r is a non-const object, and non-const forEach is used. But since the iterator is const in the second version, you can't modify the iterable.

Const correctness for value parameters

My take on it:

It's not a bad idea, but the issue is minor and your energy might be better spent on other things.

In your question you provided a good example of when it might catch an error, but occasionally you also end up doing something like this:

void foo(const int count /* … */)
{
   int temp = count;  // can't modify count, so we need a copy of it
   ++temp;

   /* … */
}

The pros and cons are minor either way.

---

Related Topics