Solution
When I pose this kind of problem, I find that
most people think the problem is on the easy side and address only
the more-usual const issues. There are, however,
subtleties that are worth knowing, hence this Item. See also the
box "const and mutable Are Your
Friends."
class Polygon
{
public:
Polygon() : area_(-1) {}
void AddPoint( const Point pt ) { InvalidateArea();
points_.push_back(pt); }
-
The
Point object is passed by value, so there is little or no
benefit to declaring it const. In fact, to the compiler,
the function signature is the same whether you include this
const in front of a value parameter or not. For
example:
int f( int );
int f( const int ); // redeclares f(int)
// no overloading, there's only one function
int g( int& );
int g( const int& ); // not the same as g(int&)
// g is overloaded
Guideline
|
Avoid const
pass-by-value parameters in function declarations. Still make the
parameter const in the same function's definition if it
won't be modified.
|
Point GetPoint( const int i ) { return points_[i]; }
-
Same comment
about the parameter type. Normally, const pass-by-value is
unuseful and misleading at best.
-
This should be
a const member function, since it doesn't change the state
of the object.
-
If you're not
returning a builtin type, such as int or long,
return-by-value should usually return a const
value. This assists client code by making sure
the compiler emits an error if the caller tries to modify the
temporary.
For example, the user might write something like
"poly.GetPoint(i) = Point(2,2);". If this were intended to
work, GetPoint() should have used return-by-reference and
not return-by-value in the first place. As we will see later, it
makes sense for GetPoint() to be a const member
function for another reason: GetPoint() should be usable
on const Polygon objects in
operator+().
Guideline
|
|
When using
return-by-value for non-builtin return types, prefer returning a
const value.
|
int GetNumPoints() { return points_.size(); }
-
Again, the
function should be const.
Incidentally, this is a good example of where
not to write a const return-by-value. A return type of
"const int" would be wrong in this case, because: (a)
returning "const int" doesn't buy you anything, since the
returned int is already an rvalue and so can't be
modified; and (b) returning "const int" can interfere with
template instantiation and is confusing, misleading, and probably
fattening.
double GetArea()
{
if( area_ < 0 ) // if not yet calculated and cached
{
CalcArea(); // calculate now
}
return area_;
}
-
Even though
this function modifies the object's internal state, it should be
const. Why? Because this function does not modify the
object's observable state. We are doing some caching here, but
that's an internal implementation detail and the object is
logically const even if it isn't physically
const.
Corollary: The member variable area_
should be declared mutable. If your compiler doesn't
support mutable yet, kludge this with a
const_cast of area_ and write a comment telling
the next person to remove the const_cast once
mutable is available—but do make the function
const.
private:
void InvalidateArea() { area_ = -1; }
-
This function
ought also to be const, if for no other reason than
consistency. Granted, semantically it will be called only from
non-const functions, since its purpose is to invalidate
the cached area_ when the object's state changes. But
consistency and clarity are important to good programming style, so
make this function const too.
void CalcArea()
{
area_ = 0;
vector<Point>:: iterator i;
for( i = points_.begin(); i != points_.end(); ++i )
{
area_ += /* some work */;
}
}
-
This member
function definitely should be const, because it does not
alter the externally visible state of the object. Note also that
this function is called only from another const member
function, namely GetArea().
-
The iterator
should not change the state of the points_ collection, so
it ought to be a const_iterator.
vector<Point> points_;
double area_;
};
Polygon operator+( Polygon& lhs, Polygon& rhs )
{
-
The parameters
should be passed by references to const, of course.
-
Again, the
return-by-value of an object should be const.
Polygon ret = lhs;
int last = rhs.GetNumPoints();
-
Because
last should never change, say so by making its type
"const int".
for( int i = 0; i < last; ++i ) // concatenate
{
ret.AddPoint( rhs.GetPoint(i) );
}
return ret;
}
(Once we make rhs a
reference-to-const parameter, we see another reason why
GetPoint() should be a const member
function.)
void f( const Polygon& poly )
{
const_cast<Polygon&>(poly).AddPoint( Point(0,0) );
}
Bonus: Using the result of the
const_cast is undefined if the referenced object was
declared as const—which it is in the case of
f(cpoly) below. The parameter isn't really const,
so don't declare it as const. It's tantamount to
lying.
void g( Polygon& const rPoly ) { rPoly.AddPoint( Point(1,1) ); }
-
This
const is not only useless, because references are already
const inasmuch as they cannot be reseated (changed to
refer to a different object), but actually illegal.
void h( Polygon* const pPoly ) { pPoly->AddPoint( Point(2,2) ); }
-
This
const is equally useless, but for a different reason:
Because we're passing the pointer by value, this makes as little
sense as passing a parameter "const int" above.
Think about it—it's exactly the same thing, because this
const merely promises that you won't modify the pointer
value. That's not a very useful promise, because the caller can't
possibly care or be affected by that decision.
(If, in looking for the "bonus" part, you said
something about this function being uncompilable—sorry, it's quite
legal C++. You were probably thinking of putting the const
to the left of the *, which would have made the function
body illegal.)
int main()
{
Polygon poly;
const Polygon cpoly;
f(poly);
This is fine.
f(cpoly);
This causes undefined results when f()
tries to cast away the const-ness of its parameter and
then modify it. See above.
g(poly);
This is fine.
h(&poly);
}
This is fine.
|
Be const-correct: const and
mutable are your friends.
I still sometimes come across programmers who
think const isn't worth the trouble. "Aw, const
is a pain to write everywhere," I've heard some complain. "If I use
it in one place, I have to use it all the time. And anyway, other
people skip it, and their programs work fine. Some of the libraries
I use aren't const-correct either. Is const worth
it?"
We could imagine a similar scene, this time at a
rifle range: "Aw, this gun's safety is a pain to set all the time.
And anyway, some other people don't use it either, and some of them
haven't shot their own feet off…"
Safety-incorrect riflemen are not long for this
world. Nor are const-incorrect programmers, carpenters who
don't have time for hard hats, and electricians who don't have time
to identify the live wire. There is no excuse for ignoring the
safety mechanisms provided with a product, and there is no excuse
for programmers too lazy to write const-correct code.
(Perhaps not strangely, the same people who skip const
tend to ignore compiler warnings, too. Illuminating, that.) In many
development shops, skipping const and ignoring compiler
warnings are "one-warning" offences, and rightly so.
Using const whenever possible makes for
safer, cleaner code. It lets you document interfaces and invariants
far more effectively than any mere /* I promise not to change
this */ comment can accomplish. It's a powerful part of
"design by contract." It helps the compiler to stop you from
accidentally writing bad code. It can even help the compiler to
generate tighter, faster, smaller code. That being the case,
there's no reason why you shouldn't use it as much as possible, and
there's every reason why you should.
Remember that the correct use of
mutable is a key part of const-correctness. If
your class contains a member that could change even for
const objects and operations, make that member
mutable. That way, you will be able to write your class's
const member functions easily and correctly, and users of
your class will be able to correctly create and use const
and non-const objects of your class's type.
It's true that not all commercial libraries'
interfaces are const-correct. That isn't an excuse for you
to write const-incorrect code, though. (It is, however,
one of the few good excuses to write const_cast.) Always
make your own code const-correct, and when you call the
broken library, you can use the appropriate const_casts
wherever necessary—preferably with a detailed comment nearby
grumbling about the library vendor's laziness and how you're
looking for a replacement product.
If you still harbor any doubts about
const, run (don't walk) and reread Item 21, "Use const
whenever possible," in Meyers98. It's for good reason
that the C language has had const since the
pre-standardization days. Back in the early 1980s, a researcher
named Stroustrup had designed this funny little language called C
with Classes, which had a funny little keyword called
readonly that was demonstrably useful for specifying
cleaner interfaces and, therefore, writing safer code. The C folks
at Bell Labs liked the idea, but preferred the name const.
When the ANSI C committee got rolling a short time later, they
adopted the same feature with the same name—and the rest, as they
say, is history (Stroustrup94, page 90).
Don't shoot yourself (or your fellow
programmers) in the foot. Always check your safety: Be
const-correct. Write const whenever possible, and
smile at the thought of cleaner, safer, tighter code.
|
That's it. Here is a revised version of the code
that corrects the const issues noted above (but does not
attempt to correct any other poor style).
class Polygon
{
public:
Polygon() : area_(-1) {}
void AddPoint( Point pt ) { InvalidateArea();
points_.push_back(pt); }
const Point GetPoint( int i ) const { return points_[i]; }
int GetNumPoints() const { return points_.size(); }
double GetArea() const
{
if( area_ < 0 ) // if not yet calculated and cached
CalcArea(); // calculate now
return area_;
}
private:
void InvalidateArea() const { area_ = -1; }
void CalcArea() const
{
area_ = 0;
vector<Point>::const_iterator i;
for( i = points_.begin(); i != points_.end(); ++i )
area_ += /* some work */;
}
vector<Point> points_;
mutable double area_;
};
const Polygon operator+( const Polygon& lhs,
const Polygon& rhs )
{
Polygon ret = lhs;
const int last = rhs.GetNumPoints();
for( int i = 0; i < last; ++i ) // concatenate
ret.AddPoint( rhs.GetPoint(i) );
return ret;
}
void f( Polygon& poly )
{
poly.AddPoint( Point(0,0) );
}
void g( Polygon& rPoly ) { rPoly.AddPoint( Point(1,1) ); }
void h( Polygon* pPoly ) { pPoly->AddPoint( Point(2,2) ); }
int main()
{
Polygon poly;
f(poly);
g(poly);
h(&poly);
}
|
