Guru of the Week #6: Const-Correctness
Herb Sutter
.--------------------------------------------------------------------.
| Guru of the Week problems and solutions are posted weekly on |
| news:comp.lang.c++.moderated. For past problems and solutions, |
| see the GotW archive at http://www.cntc.com. |
| Is there a topic you'd like to see covered? mailto:he...@cntc.com |
`--------------------------------------------------------------------'
GotW #6: Const-Correctness
Difficulty: 6 / 10
Const is a powerful tool for writing safer code, and it can help
compiler optimizations. You should use it as much as possible...
but what does "as much as possible" really mean?
Don't comment on or change the structure of this program; it's
contrived and condensed for illustration only. Just add or
remove "const" (including minor variants and related keywords)
wherever appropriate. Bonus Question: In what places are the
program's results undefined/uncompilable due to const errors?
class Polygon {
public:
Polygon() : area_(-1) {}
void AddPoint( const Point pt ) { InvalidateArea();
points_.push_back(pt); }
Point GetPoint( const int i ) { return points_[i]; }
int GetNumPoints() { return points_.size(); }
double GetArea() {
if( area_ < 0 ) // if not yet calculated and cached
CalcArea(); // calculate now
return area_;
}
private:
void InvalidateArea() { area_ = -1; }
void CalcArea() {
area_ = 0;
vector<Point>::iterator i;
for( i = points_.begin(); i != points_.end(); ++i )
area_ += /* some work */;
}
vector<Point> points_;
double area_;
};
Polygon operator+( Polygon& lhs, Polygon& rhs ) {
Polygon ret = lhs;
int last = rhs.GetNumPoints();
for( int i = 1; i <= last; ++i ) // concatenate
ret.AddPoint( rhs.GetPoint(i) );
return ret;
}
void f( const Polygon& poly ) {
const_cast<Polygon&>(poly).AddPoint( Point(0,0) );
}
void g( Polygon& const rPoly ) { rPoly.AddPoint( Point(1,1) ); }
void h( Polygon* const pPoly ) { pPoly->AddPoint( Point(2,2) ); }
int main() {
Polygon poly;
const Polygon cpoly;
f(poly);
f(cpoly);
g(poly);
h(&poly);
}
---
Herb Sutter (mailto:he...@cntc.com)
Current Network Technologies Corp. (http://www.cntc.com)
2695 North Sheridan Way, Suite 150, Mississauga ON Canada L5K 2N6
Tel 416-805-9088 Fax 905-822-3824
[ Send an empty e-mail to c++-...@netlab.cs.rpi.edu for info ]
[ about comp.lang.c++.moderated. First time posters: do this! ]
Christian Millour
Herb Sutter wrote:
> Don't comment on or change the structure of this program; it's
> contrived and condensed for illustration only. Just add or
> remove "const" (including minor variants and related keywords)
> wherever appropriate. Bonus Question: In what places are the
> program's results undefined/uncompilable due to const errors?
>
I assume that passing and returning Points by value instead of
const reference is required. also that vector is the STL vector.
> class Polygon {
> public:
> Polygon() : area_(-1) {}
>
> void AddPoint( const Point pt ) { InvalidateArea();
^^1^^
1) harmless (?) but unneeded. pt is a local copy.
> points_.push_back(pt); }
> Point GetPoint( const int i ) { return points_[i]; }
^^2^^ ^^3^^
2) potentially harmful. since vector<T>::operator[] takes an int
argument there is room for an extra unneeded int copy (hopefully
optimized away ?).
3) missing const. this method is physically const.
> int GetNumPoints() { return points_.size(); }
^^4^^
4) missing const. this method is physically const.
>
> double GetArea() {
^^5^^
5) missing const. this method is logically const. see 6) and 7).
> if( area_ < 0 ) // if not yet calculated and cached
> CalcArea(); // calculate now
> return area_;
> }
>
> private:
> void InvalidateArea() { area_ = -1; }
>
> void CalcArea() {
^^6^^
6) missing const.
> area_ = 0;
> vector<Point>::iterator i;
^^6b^^
6b) const-iterator here.
> for( i = points_.begin(); i != points_.end(); ++i )
> area_ += /* some work */;
> }
>
> vector<Point> points_;
> double area_;
^^7^^
7) missing mutable.
> };
>
> Polygon operator+( Polygon& lhs, Polygon& rhs ) {
^^8^^ ^^9^^
8,9) missing const
> Polygon ret = lhs;
> int last = rhs.GetNumPoints();
> for( int i = 1; i <= last; ++i ) // concatenate
> ret.AddPoint( rhs.GetPoint(i) );
won't work as intended (Polygons are zero-based as vectors). not a
const issue tho...
> return ret;
> }
>
> void f( const Polygon& poly ) {
> const_cast<Polygon&>(poly).AddPoint( Point(0,0) );
> }
[bonus] expect undefined behavior in some circumstances.
>
> void g( Polygon& const rPoly ) { rPoly.AddPoint( Point(1,1) ); }
^^10^^
10) compiler-defined error/warning message here. const is not
applicable to references.
>
> void h( Polygon* const pPoly ) { pPoly->AddPoint( Point(2,2) ); }
^^11^^
11) harmless (?) but unneeded.
>
> int main() {
> Polygon poly;
> const Polygon cpoly;
> f(poly);
> f(cpoly);
[bonus] could reformat your hard disk.
> g(poly);
> h(&poly);
Bradd W. Szonye
Herb Sutter <he...@cntc.com> wrote in article
<5gtok7$7...@netlab.cs.rpi.edu>...
Note: deleted 'const' or 'const_cast' marked with '@', added const marked
with '@const', added mutable marked with '@mutable'.
> class Polygon {
> public:
> Polygon() : area_(-1) {}
>
> void AddPoint( @ Point pt ) { InvalidateArea();
> points_.push_back(pt); }
The parameter pt is an rvalue; const rvalues are unnecessary and can make
writing the function itself difficult. This method affects the Polygon's
logical value and is appropriately non-const.
> Point GetPoint( @ int i ) @const { return points_[i]; }
> int GetNumPoints() @const { return points_.size(); }
These methods do not affect the logical or physical value of the polygon,
so they should be const.
> double GetArea() @const {
> if( area_ < 0 ) // if not yet calculated and cached
> CalcArea(); // calculate now
> return area_;
> }
This affects the physical but not logical state of the polygon; to fix
this:
- do not cache the area
- const_cast the object to allow modifying the area
- declare the area cache as mutable or with indirection
Since it does not affect the logical state, GetArea() should be a const
method. Older compilers will require one of the work-arounds to get it to
compile.
> private:
> void InvalidateArea() @const { area_ = -1; }
>
> void CalcArea() @const {
> area_ = 0;
> vector<Point>::iterator i;
> for( i = points_.begin(); i != points_.end(); ++i )
> area_ += /* some work */;
> }
Again, these methods do not affect the Polygon's logical state, so they
should be const. They will not compile const as-is; area_ needs fixing as
noted above.
> vector<Point> points_;
> @mutable double area_;
I've added 'mutable' to the declaration of 'area_'; some technique for
expressing area_ as a physical-only (mutable) attribute is necessary.
Without this change, CalcArea() and GetArea() may not be const, which is
really not acceptable from a design standpoint.
> };
>
> Polygon operator+( Polygon @const & lhs, Polygon @const & rhs ) {
> Polygon ret = lhs;
> int last = rhs.GetNumPoints();
> for( int i = 1; i <= last; ++i ) // concatenate
> ret.AddPoint( rhs.GetPoint(i) );
> return ret;
> }
This operator does not modify its arguments physically or logically, so the
arguments should be references to const Polygons.
> void f( @ Polygon& poly ) {
> @ poly.AddPoint( Point(0,0) );
> }
This function does modify its argument, so the 'poly' should not be const.
In this case, taking a const and casting away is a lie in the interface.
> void g( Polygon& @ rPoly ) { rPoly.AddPoint( Point(1,1) ); }
A const reference (as opposed to reference to const) is meaningless--it may
in fact be simply incorrect. (Neither the Polygon nor the reference should
be const, since the Polygon is modified and references are always const.)
> void h( Polygon* @ pPoly ) { pPoly->AddPoint( Point(2,2) ); }
Again, making rvalue paramters const is unnecessary. (Neither the Polygon
nor the pointer should be const, since the Polygon is modified and the
pointer is an rvalue.)
> int main() {
> Polygon poly;
> const Polygon cpoly;
> f(poly);
> f(cpoly);
Now that f() does not lie in its interface, f(cpoly) becomes illegal rather
than surprising.
> g(poly);
> h(&poly);
> }
By messing with the const_cast and mutable declarations, I think I've
violated the letter (if not the spirit) of the problem's requirements.
However, in real life it's one of those situations where I'd go back to my
software analyst and complain that the requirements are flawed <smile>.
--
Bradd W. Szonye
bra...@concentric.net
Phil Weiss
Herb Sutter wrote:
> class Polygon {
> public:
> Polygon() : area_(-1) {}
>
> void AddPoint( const Point pt ) { InvalidateArea();
> points_.push_back(pt); }
> Point GetPoint( const int i ) { return points_[i]; }
probably should be: Point GetPoint (int i) const;
> int GetNumPoints() { return points_.size(); }
probably should be: int GetNumPoints() const;
also, I haven't yet seen the need for object passed parameters to be
declared const (as in AddPoint and GetPoint).
> double GetArea() {
> if( area_ < 0 ) // if not yet calculated and cached
> CalcArea(); // calculate now
> return area_;
> }
>
> private:
> void InvalidateArea() { area_ = -1; }
>
> void CalcArea() {
> area_ = 0;
> vector<Point>::iterator i;
> for( i = points_.begin(); i != points_.end(); ++i )
> area_ += /* some work */;
> }
>
> vector<Point> points_;
> double area_;
As a "cached" calculation, it may be appropriate to declare area_ as
mutable.
> };
>
> Polygon operator+( Polygon& lhs, Polygon& rhs ) {
> Polygon ret = lhs;
> int last = rhs.GetNumPoints();
> for( int i = 1; i <= last; ++i ) // concatenate
> ret.AddPoint( rhs.GetPoint(i) );
> return ret;
> }
declaration should be: Polygon operator+( const Polygon& lhs, const
Polygon& rhs);
I declare all reference and pointer passed parameters as const unless
the function is intended to modify them. It usually saves me from
inadvertently modifying them.
> void f( const Polygon& poly ) {
> const_cast<Polygon&>(poly).AddPoint( Point(0,0) );
> }
This may not do what was intended. The const promise was not kept. (It
however, may be necessary on a compiler without support for mutable.)
> void g( Polygon& const rPoly ) { rPoly.AddPoint( Point(1,1) ); }
> void h( Polygon* const pPoly ) { pPoly->AddPoint( Point(2,2) ); }
const in each of these cases may not ensure the promise that the author
intended. Both functions will compile just nicely, as the const
modifies the pointer, not it's underlying object. (This is similar to
declaring parameters above as "const int".)
> int main() {
> Polygon poly;
> const Polygon cpoly;
> f(poly);
> f(cpoly);
undefined behavior here (see comment above). The comiler may have
placed cpoly in read-only memory, causing all sorts of problems by
attempting to modiy cpoly.
> g(poly);
> h(&poly);