Section 5 - Essential Ingredients in All Class Designs

You now know what a class is, what instantiating an object of the class means, what private member data means, what public instance methods are, what mutators, accessors and constructors are.  (If you don't, then you need to stop reading now and go back and reread the last few pages.  Then, if you are still unclear ask a question in the forums.  In order to understand anything going forward, including this section, you first have to know the above vocabulary solidly.)

6B.5.1 Essential Idea #1:  Constructors with Parameters Should Call Mutators

Let's take another look at the Galaxy constructor that takes two parameters:

// 2-parameter constructor
Galaxy::Galaxy(string myName, double myMag)
{
if (myName.length() > 1)
name = myName;
else
name = "undefined";
if (myMag >= -3 && myMag <= 30)
magnitude = myMag;
else
magnitude = 0.0;
}

Also, in this same class, we've seen the mutators setMagnitude() and setName():

// mutators "set" methods
bool Galaxy::setMagnitude(double mag)
{
if (mag < -3 || mag > 30)
return false;
// else
magnitude = mag;
return true;
}

bool Galaxy::setName(string theName)
{
if (theName.length() < 2)
return false;
// else we fall through
name = theName;
return true;
}

Do you notice that we are duplicating code?  This is bad.  We can make the code more efficient and less prone to error if we make use of the mutator from the constructor.  The first attempt at doing this is not going to be correct, but it is a good guess:

// 2-parameter constructor
Galaxy::Galaxy(string myName, double myMag)
{
setName(myName);
setMagnitude(myMag);
}

What's wrong with this?  It looks pretty good, right?  Instead of doing the testing manually, it calls the fellow instance methods, setName() and setMagnitude() which will take the client's data (passed in through myName and myMag) and passes them along to these mutators which will filter out any bad values.  This is almost correct.  The problem is that if values passed in are bad, then the mutator will not only reject them, but will not assign anything to the private data.  So if myMag = -234, then the call setMagnitude(myMag) will fail, return false (which we are not testing as you see) and leave magnitude undefined!  Constructors should never leave member data undefined. 

If you think that the way to fix this is by forcing the mutators to always set private data, even if bad values are passed in, then you need to think a little further.  I'll show you a better solution that allows the mutators to (correctly) not make any assignment if a bad value is passed in:

// 2-parameter constructor
Galaxy::Galaxy(string myName, double myMag)
{
if ( !setName(myName) )
name = "undefined";
if ( !setMagnitude(myMag) )
magnitude = 0.0;
}

Aha!  We are using, rather than throwing away, the return values of the mutators.  If they are true, no problem.  If they are false, we manually put a default value in the member(s).  Problem solved.

Note that for default constructors, there's no need to call the mutators because you, the class designer, have full control over the default value you assign.  There is no point in testing something for being within-range if you know what the value is ("undefined" and 0.0).  So we leave the default constructor as it is.

6B.5.2 Essential Idea #2: Class Methods Must Protect Private Data and Not Rely on Clients

Here is a common error that beginning programmers make:

A client (e.g., main()) method call:

if ( userName.length() < 2 )
gal1.setName( userName );

The definition of setName() WRONG.:

bool Galaxy::setName(string theName)
{
name = theName;
return true;
}

The beginner says "I am protecting my data in the client so I don't have to worry about it in the mutator."

Here is the concept I want you to learn today: 

Important

As class programmers, we are designing our classes for a thousand other clients that we have never seen, nor will we ever see.  Our test client is only one sample main().  There will be thousands others to come.

Therefore, there is no way we can assume anything about our client.  If the client tests for bad input, that's great.  But what if it doesn't?  If our class can crash a program because an evil client is passing it bad data, it is not the client's fault, it is our fault as class designers.  Our classes should never fail.  Therefore, we must always test every instance method which takes client parameters for bad data before we do anything.

6B.5.3 Essential Idea #3: Any Constant that Relates to the Whole Class Should be Given a Symbolic Name

Literals like "undefined" and 0.0 create maintenance problems. Every time we change them in one place, we have to change them everywhere. This can be avoided by making such values static consts in our class. Let's do that now, improving our Galaxy class even more.

class Galaxy
{
...

public:

// static constants
static const double DEFAULT_MAG;
static const string DEFAULT_NAME;

...

// default constructor
Galaxy()
{
name = DEFAULT_NAME;
magnitude = DEFAULT_MAG;
}

// 2-parameter constructor
Galaxy(String myName, double myMag)
{
if ( !setName(myName) )
name = DEFAULT_NAME;
if ( !setMagnitude(myMag) )
magnitude = DEFAULT_MAG;
}

...
};

const double Galaxy::DEFAULT_MAG = 0.0;
const string Galaxy::DEFAULT_NAME = "undefined";

Notice that for most static members, we must initialize them outside the class prototype as shown above.

With this improvement, we have one place where we would change DEFAULT_NAME or DEFAULT_MAG, and when done, all references later in the class are automatically fresh. If there is even one literal (like "undefined" and 0.0) used where these symbolic names could have been used, the entire class is broken (and points will be deducted). Unless you use the symbolic names everywhere in the class, they will not serve their purpose.

Caution

A loop counter or other trivial helper variable for a method should not be defined as a class member like this. What I described above are values which are important to the class, independent of implementation (a minimum magnitude has meaning even if we are not programming anything). But a loop counter or other helper variable only has meaning as a local variable for the method in which it is used. Even if you use the same variable name in multiple methods, do not make such variables members. Furthermore, it's especially important not to make such variables class members for the purpose of communicating their values between methods. That violates the rule about not using "globals." In this section, I am only referring to innate class constants.

Here is the entire program, with all improvements. This would get full credit if submitted as an assignment.

#include<iostream>
#include <string>
using namespace std;

// ---------------- the class prototype ---------------------------------
class Galaxy
{
private:
string name;
double magnitude;
public:

// static constants
static const double DEFAULT_MAG;
static const string DEFAULT_NAME;
static const double MIN_MAG;
static const double MAX_MAG;
static const int MIN_STR_LEN = 2;

// default constructor
Galaxy();

// 2-parameter constructor (to be discussed)
Galaxy(string myName, double myMag);

// mutators and accessors
bool setMagnitude(double mag);
bool setName(string theName);
string getName();
double getMagnitude();
};

const double Galaxy::DEFAULT_MAG = 0.0;
const string Galaxy::DEFAULT_NAME = "undefined";
const double Galaxy::MIN_MAG = -3.;
const double Galaxy::MAX_MAG = 30.;

// ------------------ the main method ---------------------------------
int  main()
{
// declare the objects
Galaxy gal1, gal2;

// try to set the data
gal1.setName("X");
gal1.setMagnitude(100);
gal2.setName("Stephan's Third");
gal2.setMagnitude(13.2);

// let's see what happened
cout << "Gal #1 name: " <<  gal1.getName() << endl;
cout << "Gal #1 mag: " <<  gal1.getMagnitude() << endl;
cout << "Gal #2 name: " <<  gal2.getName()<< endl;
cout << "Gal #2 mag: " <<  gal2.getMagnitude()<< endl;

return 0;
}

// ------------ Galaxy member functions definitions ------------
// default constructor
Galaxy::Galaxy()
{
name = DEFAULT_NAME;
magnitude = DEFAULT_MAG;
}

// 2-parameter constructor
Galaxy::Galaxy(string myName, double myMag)
{
if ( !setName(myName) )
name = DEFAULT_NAME;
if ( !setMagnitude(myMag) )
magnitude = DEFAULT_MAG;
}

// mutators "set" methods
bool Galaxy::setMagnitude(double mag)
{
if (mag < MIN_MAG || mag > MAX_MAG)
return false;
// else
magnitude = mag;
return true;
}

bool Galaxy::setName(string theName)
{
if (theName.length() < MIN_STR_LEN)
return false;
// else we fall through
name = theName;
return true;
}
// accessor "get" methods
string Galaxy::getName()
{
return name;
}
double Galaxy::getMagnitude()
{
return magnitude;
}

/* ------------------ Paste of Run from Above Program ----------

Gal #1 name: undefined
Gal #1 mag: 0
Gal #2 name: Stephan's Third
Gal #2 mag: 13.2
Press any key to continue . . .

---------------------------------------------------------------- */

 

You will probably see examples in the modules that follow which violate this rule. Call any such instances to my attention in the typos forum so I can fix them. The original modules were written under some time pressure, and I did not fully vet all the examples. My apology in advance.

Prevent Point Loss

guy with blackboard