Sign in to follow this  

Help me understand the protected keyword

This topic is 3933 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I have an abstract base class which I'll call Human. It has two derived classes, call them Killer and Victim. It also has a protected member _protIntArray. Now, at one point, Killer has to take a pointer to a Victim and do something to its _protIntArray, thusly:
void Killer::kill (VictimListWithExtraInfo* victs) {
  victs->theVictimList[0]->_protIntArray[0] += 5;
}
If I understand the documentation correctly, Killer, being a subclass of Human, should be able to access protected members of Human through a pointer to another subclass of Human. Right? (I'm getting this from here.) But my compiler (gcc 3.2.3) gives me this: Human.hh:70: `IntArrayWrapper Human::_protIntArray' is protected Killer.cc:68: within this context What am I misunderstanding, here?

Share this post


Link to post
Share on other sites
Hum. I tried that with an explicit cast, thus:


void Killer::kill (VictimListWithExtraInfo* victs) {
((Human*) (victs->theVictimList[0]))->_protIntArray[0] += 5;
}


but I'm still getting the same error.

Share this post


Link to post
Share on other sites
Killer being a subclass of human means that killer does indeed inherit the protected member. This means that an instance of killer has the member _protIntArray. It can access it's own _protIntArray. It can not access the protected or private members of another instance.

Share this post


Link to post
Share on other sites
Quote:
Original post by shengshwi
Killer being a subclass of human means that killer does indeed inherit the protected member. This means that an instance of killer has the member _protIntArray. It can access it's own _protIntArray. It can not access the protected or private members of another instance.


while the behavior you are seeing seems like what shengshwi is describing ... that is not he behavior I am used to or believe is correct.

if this is correct:


class A
protected:
int i;
public:
void DoSomething(A &rhs)
{
i += rhs.i; // accessing another instances i directly
}


why wouldn't this be correct:

class B : public A
public:
void DoSomethingElse(B &rhs)
{
i += rhs.i; // accessing another instances i directly
}


I thought a derived class could internally do with protected members from its base, the same thing it can do with private members of itself?

Is this not correct?

Share this post


Link to post
Share on other sites
also, if it cannot access "rhs.i", it wouldn't be able to access a protected method on rhs either "rhs.SomeProtectedFunc()" would fail too, because there is no difference in C++ between data members and method members (that I am aware of).

I guess if the access to rhs.??? was inside of the function you put in the base class, then it could work even if the above fails.

Share this post


Link to post
Share on other sites
Quote:
Original post by Xai
I thought a derived class could internally do with protected members from its base, the same thing it can do with private members of itself?

Is this not correct?


It is. However, based on his question and code, I believe he has a setup like this:


class A
{
protected:
int i;
};

class B : public A
{
};

class C : public A
{
public:
void doSomething(B b)
{
b.i = 5; //Error here
}
};



Inherited protected members remain protected; as such, other classes can't access them, even if the two classes are part of the same inheritance hierarchy.

Share this post


Link to post
Share on other sites
I suspect the OP has a hierarchy like this



class Human
{
protected:

int value;
};

class Killer : public Human
{
public:

void updateValue()
{
value++;
}
};

class Victim : public Human
{
public:

void updateValue()
{
value++; // ok. updating local value
}

void updateKillerValue(Killer& killer)
{
killer.value++; // error. Victim does not inherit Killer so this is not protected on our behalf.

// This does not work either even if we do inherit Human
Human* h = (Human*)&killer;
h->value++;

}
};




I have to say Im a bit confused myself.

Share this post


Link to post
Share on other sites
Ok, so now I understand what's going on. Is there any way, though, to give the whole inheritance hierarchy access to each others' protected parts, without going whole hog and making them public? And preferably without horrible kludges like declaring them all friends of each other.

Share this post


Link to post
Share on other sites
If you really want to do it, you can cast the ptr to a killer:

void Killer::kill (VictimListWithExtraInfo* victs) {
((Killer*) (victs->theVictimList[0]))->_protIntArray[0] += 5;
}

Really what you should do is use a clean object oriented approach with getters and setters. Something like this:

void Killer::kill (VictimListWithExtraInfo* victs) {
(victs->theVictimList[0])->IncrementProtIntArrayAt(0,5);
}

void Human::IncrementProtIntArrayAt(int indx, int value){
_protIntArray[indx] += value;
}

Share this post


Link to post
Share on other sites
Quote:
Original post by King of Men
Ok, so now I understand what's going on. Is there any way, though, to give the whole inheritance hierarchy access to each others' protected parts, without going whole hog and making them public? And preferably without horrible kludges like declaring them all friends of each other.


Not really. I don't think that is such a great idea anyway, things should be kept as private as possible. You could expose some of the actions of the Human class as protected static methods, but thats a bit messy (why not just have public operations to do... whatever that line actually does ).
(note:)
I cannot tell what the field "_protIntArray" does. Thats a terrible name. It tells me nothing that I couldn't figure out by browsing to the header file ( assuming I had a copy, of course [smile] ). It should be named according to what this int array represents.

Like so:

class Human {
protected:
// real name pending =)
std::vector<int> _protIntArray;

// again a name would be nice
static void incrementFirstElementOfIntArrayForSomeReason( Human &human )
{
human._protIntArray.front() += 5;
}
};

class Victim : public Human {};

class Killer : public Human {
public:
// for simplicities sake:
void kill( Victim &victim )
{
Human::incrementFirstElementOfIntArrayForSomeReason( victim );
}
};


Share this post


Link to post
Share on other sites
Quote:
Original post by rip-off
I cannot tell what the field "_protIntArray" does. Thats a terrible name. It tells me nothing that I couldn't figure out by browsing to the header file ( assuming I had a copy, of course [smile] ). It should be named according to what this int array represents.


Of course it's not named that in my code! It's a descriptive name for purposes of describing the problem I'm having, where protIntArray is exactly what the variable is, to wit, a protected integer array. In my code it's doing something quite different from being a protected integer array, but that's not the correct level of description for describing a compiler problem; if I were having a logic problem, I'd use the internal name.

Quote:
Really what you should do is use a clean object oriented approach with getters and setters. Something like this:


void Killer::kill (VictimListWithExtraInfo* victs) {
(victs->theVictimList[0])->IncrementProtIntArrayAt(0,5);
}



That's just an obfuscated way of making the array public. If I try making IncrementProtIntArray a protected method, I'll have exactly the same problem as before, since Killer won't be able to access Victim's method. And if I make it public, I might as well go whole hog and make the array public in the first place, since anyone who wants to will be able to modify it. The point is that I want this functionality encapsulated to my Human-descended hierarchy.

Share this post


Link to post
Share on other sites
Quote:
Original post by King of Men
Quote:
Original post by rip-off
I cannot tell what the field "_protIntArray" does. Thats a terrible name. It tells me nothing that I couldn't figure out by browsing to the header file ( assuming I had a copy, of course [smile] ). It should be named according to what this int array represents.


Of course it's not named that in my code! It's a descriptive name for purposes of describing the problem I'm having, where protIntArray is exactly what the variable is, to wit, a protected integer array. In my code it's doing something quite different from being a protected integer array, but that's not the correct level of description for describing a compiler problem; if I were having a logic problem, I'd use the internal name.



Sorry, but I have seen variable names like that before. It just made it hard to come up with a generic example.

But what about the protected static function, does that suit your needs? I'll be the first to admit its a bit hacky, but anything that tries to subvert the access protection mechanism of c++ will be I suppose.

Share this post


Link to post
Share on other sites
Quote:
Original post by King of Men
I have an abstract base class which I'll call Human. It has two derived classes, call them Killer and Victim. It also has a protected member _protIntArray. Now, at one point, Killer has to take a pointer to a Victim and do something to its _protIntArray, thusly:


void Killer::kill (VictimListWithExtraInfo* victs) {
victs->theVictimList[0]->_protIntArray[0] += 5;
}


OK, so it looks something like this (with some ugly things renamed)?


class Human {
protected:
int magic[42];
};

class Victim : public Human {};

struct ShootingGallery {
std::vector<Victim*> victims;
int extra_info;
};

class Killer : public Human {
public:
void kill(ShootingGallery* s) {
s->victims[0]->magic[0] += 5;
}
};


OK, well, then, several questions immediately spring to mind...

- Apparently the ShootingGallery is holding its Victims by pointer. Is this for polymorphism (i.e. it actually has a container of Human*'s, and Killers are themselves capable of being targeted)? If so, have you considered the use of smart pointers, or boost ptr_containers (such as ptr_vector)? If not, why not just store the Victims by value?

- Why is the ShootingGallery passed in via pointer? We ought to use a reference instead, surely?

- The work is divided wrongly. Repeated arrow operators are a sign of violation of the Law of Demeter.

Rearranging that, we get something like:


class Human {
public:
void takeDamage() { magic[0] += 5; }
private:
int magic[42];
};

class Victim : public Human {};

class ShootingGallery {
std::vector<Victim> victims;
int extra_info;
public:
void wound(int who) const {
victims[who].takeDamage();
}
};

class Killer : public Human {
public:
void kill(const ShootingGallery& s) const {
s.wound(0);
}
};


IMX, need for the 'protected' keyword is fairly rare, even when you are using inheritance and runtime polymorphism.

Share this post


Link to post
Share on other sites
I'm not storing the Victims by value because the list of Victims is a temporary subset of my pool-of-all-Victims. I've got code elsewhere that makes a list of eligible victims, calculates the extraInfo about this particular list, and passes that to the Killer. It seems to me that this is just the kind of thing pointers were invented for; you've got things stored somewhere, and you need access to them so they can be modified.

Using a reference is a good idea. For the law of Demeter, I must say I'd never heard of it, but it's not a bad idea. However, my ShootingGallery struct is basically just a wrapper around an std::vector<Victim*>; replicating the whole Victim interface to Killer just to avoid some extra arrows strikes me as a spot of overkill. How about I give it an [] operator to wrap the vector[] instead? That would prettify the code.

Share this post


Link to post
Share on other sites
Quote:
Original post by King of Men
I'm not storing the Victims by value because the list of Victims is a temporary subset of my pool-of-all-Victims. I've got code elsewhere that makes a list of eligible victims, calculates the extraInfo about this particular list, and passes that to the Killer. It seems to me that this is just the kind of thing pointers were invented for; you've got things stored somewhere, and you need access to them so they can be modified.


Sure; that's a perfectly good use of pointers too (and another one that came to mind, but the thought didn't fully form). These are "weak pointers", in case you ever need to study the CS literature. (When you work with boost::shared_ptr, you will sometimes also need to use boost::weak_ptrs so that you can indicate which pointers are "important" - otherwise the weak pointers can result in memory leaks by "holding onto" things that aren't actually in use and creating cyclic structures.)

I'm sort of interested in what the extraInfo is, now, though. :)

Quote:
Using a reference is a good idea. For the law of Demeter, I must say I'd never heard of it, but it's not a bad idea. However, my ShootingGallery struct is basically just a wrapper around an std::vector<Victim*>; replicating the whole Victim interface to Killer just to avoid some extra arrows strikes me as a spot of overkill.


It wouldn't replicate the whole interface; only the part the Killer cares about. And it wouldn't *replicate* it, anyway; it would *delegate to* the Victim.

Quote:
How about I give it an [] operator to wrap the vector[] instead? That would prettify the code.


Sure; that, in combination with doing the addition from within the Victim, is probably about as good. Except I would want the operator to dereference as well, so that we can avoid exposing ugly pointers to the client code. And at that point, calling it operator[] is getting pretty tricky.

We get:


class Human {
public:
void takeDamage() { magic[0] += 5; }
private:
int magic[42];
};

class Victim : public Human {};

class ShootingGallery {
// weak pointers.
std::vector<Victim*> victims;
int extra_info;
public:
Victim& victim(int who) const {
return *victims[who];
}
};

class Killer : public Human {
public:
void kill(const ShootingGallery& s) const {
s.victim(0).takeDamage();
}
};


It still violates the LoD itself (by having the Killer "reach into" the Shooting Gallery to grab a Victim, and then manipulate the Victim - instead of letting the ShootingGallery "play with its own toys"), but it avoids a lot of the problems that the LoD is meant to guard against, and handles pointers in a pretty way. I think single-level, minor violations like this are usually OK. It becomes a matter of whether you can justify it in terms of object responsibilities; a ShootingGallery is "mostly just a container" which makes this look better.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
I'm sort of interested in what the extraInfo is, now, though. :)


It's basically just information about all-the-victims, such as their total points value. In principle Killer could calculate it by just iterating over the victims, but it fell much more naturally into the make-a-list-of-victims code.

Quote:
It wouldn't replicate the whole interface; only the part the Killer cares about.


That's what I meant by the 'interface to Killer'. :)

Quote:
And it wouldn't *replicate* it, anyway; it would *delegate to* the Victim.


I must say the difference strikes me as semantic. I'd still have to physically type the interface code, and maintain it later on; and god forbid that my Victim interface-to-Killer should change.

Share this post


Link to post
Share on other sites

This topic is 3933 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this