const data members, yes or no?

What’s wrong with this class?

class Point
{
private:
    int const   x_;
    int const   y_;

public:
    Point( int x, int y ): x_( x ), y_( y ) {}
    int x() const { return x_; }
    int y() const { return y_; }
};

Well, maybe nothing! :-) It depends on whether the effect of having those const data members is intentional or not, in short, whether the code expresses the programmer’s intent. The effect is to allow copy construction while disallowing copy assignment:

int main()
{
    Point   p1( 100, 200 );
    Point   p2 = p1;        // OK

    p2 = p1;                // !Nope.
}

If the programmer’s intent was to disable copying in general, perhaps to avoid slicing problems or to avoid a pointer member being copied, then instead of the const data members the class should make the copy constructor and the copy assignment operator non-public, e.g. private:

class Point
{
private:
    int x_;
    int y_;

    Point( Point const& other );                // No such.
    Point& operator=( Point const& other );     // No such.

public:
    Point( int x, int y ): x_( x ), y_( y ) {}
    int x() const { return x_; }
    int y() const { return y_; }
};

If, on the other hand, the programmer’s intent with the const members was precisely to allow copy construction while disallowing assignment, then the original implementation is technically OK. It’s a somewhat unclear way to do it, and I’d have preferred to explicitly have a non-accessible assignment operator, but it’s then technically OK. But why would one want that?

Well, one reason is the same as the reason for adding const to just about anything where it can be added: the more constraints there are on the variables’ values, the more you know about them, and the easier the code is to analyze and understand. And with assignment prohibited you know that the value of any variable of that type (at least if an instance does not refer to shared stuff) is not going to change, just as if it were const.

This is Good™. It may save a lot of work, and it centralizes the decision of “no value changes”. But on the other hand it might seem that all those const data members are unnecessary and makes it necessary to infer that the class prohibits assignment, when that could very simply have been expressed directly:

class Point
{
private:
    int x_;
    int y_;

    Point& operator=( Point const& );   // No such.

public:
    Point( int x, int y ): x_( x ), y_( y ) {}
    int x() const { return x_; }
    int y() const { return y_; }
};

And one might think that the above code, for the purpose of prohibiting assignment so as to improve the clarity of client code,

  • is more clear,
  • avoids sillywarnings from one particular sillywarning-enthused compiler, and
  • is generally less to type.

And I believe that all this is true, but there is one more aspect to consider, a rather important one!, as exemplified by this incorrect variant where the hypothetical programmer just copied and pasted the Point code and forgot to initialize the new z_ member:

// !Intentionally incorrect, not detected by compiler.
class Point3D
{
private:
    int x_;
    int y_;
    int z_;

    Point3D& operator=( Point3D const& );   // No such.

public:
    Point3D( int x, int y ): x_( x ), y_( y ) {}
    int x() const { return x_; }
    int y() const { return y_; }
    int z() const { return z_; }
};

At highest warning level g++ warns about “’p.Point3D::z_’ is used uninitialized”, but MSVC 9.0 does not. However, with data members as const they must be initialized, by the language rules, and so any failure to initialize all will be caught at compile time. I.e., …

// !Intentionally incorrect, and detected by compiler.
class Point3D
{
private:
    int const   x_;
    int const   y_;
    int const   z_;

    Point3D& operator=( Point3D const& );   // No such.

public:
    Point3D( int x, int y ): x_( x ), y_( y ) {}
    int x() const { return x_; }
    int y() const { return y_; }
    int z() const { return z_; }
};

So, this is a main reason for using const data members: that any failure to initialize will be detected at compile time.

Of course, it does imply unassignability, which is also generally Good, but which may in some cases be a problem. And unassignability is a pretty heavy constraint to impose on all derived classes and all classes having a member of your class. And so the question of whether to use const data members ends up where many such questions end up: it’s an engineering decision. :-)

However, I think that if one uses a const data member then it might be a good idea to make all data members const, for consistency, and to explicitly make the copy assignment operator private and unimplemented, as shown above. This tells a reader of the code (plus a certain compiler) that it’s intentional. And it might just be easier to grok.

About these ads

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s