Scott Meyers Effective C++ 3rd Ed. Item 22 Declare members private

Pages: 12
I have just been ruminating over this, and have trouble understanding this item, just in regard to what he is saying about protected members - what he says about public members I am fine with. So far I have read up to Item 27.

For those that don't have the book, basically he is saying that: the arguments against public data members apply equally to protected members; and Protected members are a problem because of the amount of code that can be broken when one needs to change a protected member.

I wish to say that I am reticent about arguing against a renowned expert author with a Ph.D. in Computer Science :) It is not so much me arguing per se, but more to put some questions to benefit my education and incrementing my ThingsLearnt variable quite a lot!!

The problem I have with this is it seems to go against the purpose of having protected members being effectively private in derived classes. If members are private in a base class, derived classes cannot have access to them, having public access functions in the base class does not help when one has a derived object. How to remedy this - especially if one is modelling real world situations?

Would the problem of being able to cope with changes to protected members, be better handled by making the data member a class of it's own, with it's own interface? Then the member could be public (as it has it's own interface) and inherited, any changes needed could be made internally to the member class. If the data member class now holds more info, then more interface functions can be added. In doing this, the data member's internal variables would be private - it is probably unlikely in this case that one would derive further classes from this. Is this really what he is saying?

Here is some code to explain what I was thinking - there is a lot stuff left out:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
class CNutrition {
private:
//variables to cover energy, fat, protein, carbs, salt, vitamins etc.
public:
//ctors & dtors etc
void DisplayAmountPerServe();
//other interface functions here
};

class CHumanFood {
protected:
CHumanFood(); //can't make an obj of this type
double m_Weight;//should make this a class to help future proof it

public:
CNutrition Nutrition;//has it's own interface
double getWeight();

};

class CFuitAndVeg : public CHumanFood {
protected:
std::string m_Variety;//should make this a class to help future proof it
FuitAndVeg(); //can't make an obj of this type

public:
//functions to access data
};

class CFruit : public CFuitAndVeg {
//this class is needed for generic function declarations like this:
// MakePie (CPastry *, CFruit*);
protected: 
CFruit(); //can't make an obj of this type
};

class CApple : public CFruit {
public:
CApple(); //make various apples with this class

};

//similar arrangements for veges, meat, bakery etc
//bakery might have more functions like this:
// MakePie (CPastry*, CVege*);
// MakePie (CPastry*, CMeat*);
// MakePie (CPastry*, CVege*, CMeat*); 


So now if one wants to change the underlying structure of CNutrition to include things like % of Recommended Daily Intake (%RDI) and grams per 100 grams, as well as the existing grams per serving (CNutrition::InsertGramsPerServeInfo), then these extra interface functions can be added.

With the m_Weight member, maybe we want to add the ability to specify different units such as KG, grams, Pounds, Ounces say, and allow conversions between them for output. Same story - make it a class from the start to avoid breaking code later.

Then there is the m_Variety name, often these have 4 digit numbers associated with the name, so make this a class as well.

I have also been ruminating on the Gang of Four Design Patterns book, so the other problem I have is with the idea of preferring Composition / Aggregation over inheritance. I guess it only applies in certain situations - these are the issues I am having:

First, it ruins polymorphism - that is the ability to do this say:

1
2
//A variety of different fruit
Bakery::MakePie (CPastry* Pastry, CFruit* Fruit){}


Can I conclude that it is OK to have an arbitrarily large inheritance tree if it is being used for the purpose of polymorphic function calls like this example of a Command Design Pattern?

1
2
3
//A Tile is occupied by a CEnemy and / or a CResource of some kind
//each of these classes has lots of derived classes
CCommandMgr::Execute(CAction* Action, CPlayer* Player, CWeapon* Weapon, CTile* Tile){} 


Second, if a class has more than 1 child at a particular level in the tree, then it ruins the idea of inheritance of data members. One shouldn't have to specify the composite member for each child at that level. Is that not one of the reasons why the idea of inheritance was invented? With member functions, one can write them using templates, so that they become algorithms - which is great, but we have to remember about inheritance of interface too.

Some one else also said (possibly helios)that it is a bad idea to have atomic objects (such as points in a CAD system say) as being part of a large inheritance tree - rather have them as concrete classes. I can see the sense in that good advice. I can also see the advantages of the Composite Design Pattern to build complex objects.

So, what are the situations where composition is a good idea given the other considerations? Or have I identified some exceptions to the rule - maybe there are others I haven't thought of?

What do people think about having a moderated forum, where members can put forward different examples of Design Patterns, or does such a thing exist already? I may need to follow my own advice to others & look on Google or wiki :D

Any way I look forward to your replies - Cheers.
> having public access functions in the base class does not help when one has a derived object.
Not sure what you are trying to say.
If you made a function public, anyone may have access to it.
You could make the function protected, so it can only be called from derived objects.

1
2
3
4
5
6
7
8
9
class CHumanFood {
protected:
   CHumanFood(); //can't make an obj of this type
   double m_Weight;//should make this a class to help future proof it

public:
   CNutrition Nutrition;//has it's own interface
   double getWeight();
};
`CNutrition' does not know (or shouldn't know) anything about `CHumanFood'.
By instance, ¿how does it relate with the weight? You end transferring that logic to the client code.


> if a class has more than 1 child at a particular level in the tree, then it ruins the idea of inheritance of data members
I don't get it.
http://stackoverflow.com/questions/2170688/private-virtual-method-in-c
If the function should not be called by the deriving class but may overridden, make it private instead of protected. It can still be overridden/implemented because it is virtual.

This is why private members are visible in header files in C++, even if they are not accessible.
Last edited on
@L B
You suggestion is really interesting, but I was meaning protected data members. May be I am not fully understanding how it could be used in this situation. Thanks for your reply which ever way it is.


ne555 wrote:

Not sure what you are trying to say.
If you made a function public, anyone may have access to it.
You could make the function protected, so it can only be called from derived objects.


Making things protected, is what Scott Meyers is objecting to, and is the theme of my question. But more about protected versus private data members rather than functions. I normally have have protected data members, with appropriate public functions to deal with the data. I thought this was the normal way of doing things, but Meyers is objecting to it.

My other point was: Would it be better to put data members into their own class (e.g. CNutrition), to solve the problem of breaking code when a data member changes in some way? The possibility of breaking code by changing / removing it was Meyers' motivation for his advice. The other thing was that Meyers did not provide a solution of what to do instead. I can see how removing a data member can pose big problems, but how to implement things involving inheritance without protected data members?

I don't see that having to call base class functions to deal with some of the derived classes data is a viable solution - well it is messy at least. It doesn't help the original problem of breaking code - instead it turns the code into an unnecessary mess. Why not use inheritance - isn't that what it is there for?

`CNutrition' does not know (or shouldn't know) anything about `CHumanFood'.


It doesn't, so I am not sure what you mean? I want to be able to provide nutrition info for each type of food. It is it's own class to try and prevent problems something about Nutrition changes. As should the weight & variety variables.

By instance, ¿how does it relate with the weight? You end transferring that logic to the client code.


m_weight is there so that it is inherited to all the foods - so we can have :

1
2
MyApple.getWeight; //not good to have a getter like that 
MySteak.getWeight;


or even better a class function in a derived class like CApple or CPumpkin can make use of the m_Weight variable:

std::cout << "The weight of the Pumpkin is " << m_Weight << std::endl;


If that is all the function does, then a generic version of it should be in the base class, to be inherited by all.

This is all just ordinary inheritance.

ne555 wrote:
> if a class has more than 1 child at a particular level in the tree, then it ruins the idea of inheritance of data members
I don't get it.


TheIdeasMan wrote:
One shouldn't have to specify the composite member for each child at that level. Is that not one of the reasons why the idea of inheritance was invented?


Say we had the same classes as in the Food example, but we removed the inheritance, and used composition / aggregation instead. Now we have to place a CHumanFood composite member into each of CFruitVeg, CMeat, CFish, CShellFish, CBakery -the list goes on and on (think of all the categories of food in a supermarket). At the next level and all the other levels, the same story. Messy - very messy & error prone. As well as that it ruins the polymorphism, because the classes are no longer inherited. I don't see why composition should be preferred over inheritance as a general statement - what are the exceptions to that rule?

It seems to me that having a large inheritance tree is OK for the purpose of having generic polymorphic function calls.
Composition seems bad when there are multiple child classes at a particular level in an inheritance tree.

So when is it a good idea to use composition?
Last edited on
TheIdeasMan wrote:
I normally have have protected data members, with appropriate public functions to deal with the data. I thought this was the normal way of doing things, but Meyers is objecting to it.
Data members should be private - making them protected allows a deriving class to mess with them behind your back.
L B wrote:

Data members should be private - making them protected allows a deriving class to mess with them behind your back.


But this goes back to the root of the original question: How does that work when there is inheritance?

I do want to inherit the data members - like the m_Weight member variable - I want that to be available to all of the derived classes. It is essentially private in all the derived classes, so only member functions can have access to it - which is what I want.

Again the original question: If a data member is private in a base class, how to provide access to it in a derived class without making it protected in the first place? Or another way: What to do instead?
You add protected accessor methods to the base class. Then you can call those accessors from the derived class.
Last edited on
That's seems like unnecessary effort to me. I don't see what the benefit is. The base class shouldn't care what its derived classes do with its data, IMO. For me, the base class is there for three things:

1. To provide a uniform interface (e.g. all objects derived from Actor have a Walk method)
2. To allow like objects to be stored and treated generically (e.g. objects of type Dog and Cat can both be stored in a vector of Animals)
3. To avoid code duplication (e.g. in my game engine, everything derives from GameObject which has a Draw method that automatically loads the object's texture (or finds it in a list if it's already been loaded) and draws it at the correct co-ordinates, but the Area class, which has an inventory, overrides the Draw method to draw all the objects in its inventory and then calls the base Draw method)

For me, the base class is a slave to the derived classes, so it's not allowed to keep secrets.
Last edited on
You are now making implementation details part of your public interface for any deriving class. This also contradicts your third point.
Do you mean public to the derived classes, or to the outside world? The former is sort of what I want: the base class exists to serve the derived class, so the derived class can do what it wants with it. The latter isn't the case, since I use protected members; I just use private members in base classes sparingly.

I don't see what contradicts my third point.
Unless the deriving classes know to use the helper functions in the base class, the deriving classes will suddenly be responsible with the private implementation of the base class and will have to rewrite code for each deriving class. And what if the base class' private implementation changes? You have to rewrite all the deriving classes too.
Try to change your `GameObject' class now.
The idea is that the logic to affect the base class members it is better done in the base class.

> The base class shouldn't care what its derived classes do with its data
http://en.wikipedia.org/wiki/Liskov_substitution_principle
_ Preconditions cannot be strengthened in a subtype.
_ Postconditions cannot be weakened in a subtype.
_ Invariants of the supertype must be preserved in a subtype.
_ History constraint
L B wrote:
Unless the deriving classes know to use the helper functions in the base class, the deriving classes will suddenly be responsible with the private implementation of the base class and will have to rewrite code for each deriving class

Well that's why the base class provides those functions in the first place.

what if the base class' private implementation changes? You have to rewrite all the deriving classes too.

Accessor methods have the same problem, though. What's the difference between changing
1
2
3
protected:
    std::string name;
    coords position;

and
1
2
3
4
5
protected:
    std::string get_name();
    void set_name(const std::string&);
    coords get_position();
    void set_position(const coords&);

?

It's not like an Actor class is going to one day no longer need to have a position and a name, anyway. Stuff that is only needed by a single function can be kept private, I just don't see the point in hiding data and then providing code to access it anyway. I know that it means you can regulate the access, but that shouldn't be necessary in the relationship between a base and derived class. I exaggerated when I said that base classes aren't allowed to keep secrets; in truth, I do keep small, unimportant details private.
+3 chrisname

I agree entirely, and this why I am struggling with Meyers' assertion of always having private data members, but not protected members.

@rapidcoder

Thanks for your reply, It answers the basic question in that it provides a way of accessing private members of a base class, but chrisname is right for several reasons:

1. It doesn't fix the problem of breaking code in derived classes when something changes.
2. It is heaps of extra work for no actual gain, each and every derived class must now have functions to access data - that is much worse than having data members protected in the first place.
3. The accessor functions targeting a class at a particular level in the inheritance tree will be exactly the same, just repeated everywhere.
4. One may as well not have any inheritance at all.
5. Code in member functions is complicated by the function call, rather than having direct access to the protected variable.

I should mention that I haven't finished reading the book yet, may be there is a solution later on - templates possibly?

With the composition, may be Meyers' just meant composition where needed - not everywhere:

1
2
3
4
5
6
7
class CPoint3D {};

class CCircle {

CPoint3D CentrePt;  //composition - you wouldn't inherit this
double Radius;
};


I may have just answered my own question about the composition, but I am still stumped on the protected issue. I thought that I knew this stuff, it seems like the artist is being told not to use paint brushes any more for doing paintings.
@ne555

I didn't see your post while I was writing mine.

I am OK with your suggestions, but how would you organise the Food example I had earlier?

On a slightly different note, about 2 or 3 months back someone (possibly kbw, ModShop, Moschops or CatFish) wrote a reply that listed the different reasons one might use a class for. I tried searching for it, but all these guys post quite a bit so I haven't found it yet. I am hoping that the person might remember making the post, and provide a link to it? That would be grand.
chrisname wrote:
Accessor methods have the same problem, though. What's the difference between changing
1
2
3
protected:
    std::string name;
    coords position;
and
1
2
3
4
5
protected:
    std::string get_name();
    void set_name(const std::string&);
    coords get_position();
    void set_position(const coords&);
?
The difference is that with the second version, the actual internal representation doesn't matter. In the first version, what if you wanted to change it to use a c-style string or something else instead? What if you needed to change it to query a library or network connection?
TheIdeasMan wrote:
3. The accessor functions targeting a class at a particular level in the inheritance tree will be exactly the same, just repeated everywhere.
No, only the base class must provide the accessors to mess with the base class stuff. A deriving class can't make the protected members private, and doesn't need to provide its own. if it does, you can use the scope resolution operator.
TheIdeasMan wrote:
4. One may as well not have any inheritance at all.
Are you 'for' or 'against' chrisname?? This point is against him.
TheIdeasMan wrote:
5. Code in member functions is complicated by the function call, rather than having direct access to the protected variable.
You are not the compiler. You do not need to worry about that.
Last edited on
Yes, getters and setters are evil.

You wouldn't give your wallet to the paperboy.
Even if you buy what she wants, you wouldn't give your wallet to your daughter.
Would you let the wallet float in front of you for anyone to take it? Or would you let people ask to do something to your wallet so you can decide? What if your wife needs some money? That guys wants change for a 10?
Last edited on
I don't have a wallet ;)

> What if your wife needs some money?
she could work

> That guys wants change for a 10?
coin jar, or direct him to a kiosk
chrisname wrote:
Accessor methods have the same problem, though. What's the difference between changing

1
2
3
protected:
    std::string name;
    coords position;


and

1
2
3
4
5
protected:
    std::string get_name();
    void set_name(const std::string&);
    coords get_position();
    void set_position(const coords&);


?


The difference is that you may change the first without changing the interface in the second. In other words, the interface is more stable. We wanted to stop storing the string in our base class and use an iterator into an existing container? With the first code, the interaction between derived and base class changes. In the second code, it does not.

I don't know why everyone seems to have the mentality that getters and setters are bad. They're not. Designs where getters and setters are the only methods of a class should probably be suspect, but that doesn't make getters and setters bad.

In fact, getters and setters are good because they increase encapsulation.

TheIdeasMan wrote:
2. It is heaps of extra work for no actual gain, each and every derived class must now have functions to access data - that is much worse than having data members protected in the first place.


I think you're looking at this the wrong way. You break encapsulation in the base class if you let the derived class manipulate the base class' data directly. If the derived class needs to access the base class data, then the base class interface is not complete -- the design is just as suspect as if the public interface was only getters and setters.

If the derived class has access to base class data, then we've increased the amount of code we have to look at to determine that the data is being handled correctly. One might notice a parallel to public data members (or global variables) here.

It's as Scott says in the book. The arguments against public data work equally well against protected data.
Last edited on
Pages: 12