Style Case Studies: Construction Unions - Comments
(Page 5 of 9 )
union {
int i;
unsigned char buff[max(sizeof(LIST),sizeof(STRING))];
} U;
void cleanup();
};
That’s it for the main class definition. Moving on, consider the three parallel accessor functions:
inline int& MYUNION::getint()
{
if( currtype==_INT ) {
return U.i;
} else {
cleanup();
currtype=_INT;
return U.i;
} // else
}
inline LIST& MYUNION::getlist()
{
if( currtype==_LIST ) {
return *(reinterpret_cast<LIST*>(U.buff));
} else {
cleanup();
LIST* ptype = new(U.buff) LIST();
currtype=_LIST;
return *ptype;
} // else
}
inline STRING& MYUNION::getstring()
{
if( currtype==_STRING) {
return *(reinterpret_cast<STRING*>(U.buff));
} else {
cleanup();
STRING* ptype = new(U.buff) STRING();
currtype=_STRING;
return *ptype;
} // else
}
A minor nit: The // else comments add nothing. It’s unfortunate that the only comments in the code are useless ones.
Guideline: Write (only) useful comments. Never write comments that repeat the code; instead, write comments that explain the code and the reasons why you wrote it that way.
More seriously, there are three major problems here. The first is that the functions are not written symmetrically, and whereas the first use of a list or a string yields a default-constructed object, the first use of int yields an uninitialized object. If that is intended, in order to mirror the ordinary semantics of uninitialized int variables, then that should be documented; because it is not, the int ought to be initialized. For example, if the caller accesses getint and tries to make a copy of the (uninitialized) value, the result is undefined behavior—not all platforms support copying arbitrary invalid int values, and some will reject the instruction at run-time.
The second major problem is that this code hinders const-correct use. If the code is really going to be written this way, then at least it would be useful to also provide const overloads for each of these functions; each would naturally return the same thing as its non-const counterpart, but by a reference to const.
Guideline: Practice const-correctness.
The third major problem is that this approach is fragile and brittle in the face of change. It relies on type switching, and it’s easy to accidentally fail to keep all the functions in sync when you add or remove new types.
Stop reading here and consider: What do you have to do in the published code if you want to add a new type? Make as complete a list as you can.
 This chapter is from Exceptional C++ Style, by Herb Sutter (ISBN 0201760428, copyright 2005. All rights reserved. It is reprinted with permission from Addison-Wesley Professional). Check it out at your favorite bookstore today.
Buy this book now. |
Next: Adding New Types >>
More Code Examples Articles
More By Addison-Wesley/Prentice Hall PTR
/ 8 
