Aggregating ABC's and pointer correctness

[Tigera]

Greetings,

I've been reading Scott Meyer's book, "Effective C++", and I think
that I've confused myself terribly. I have a class like this:

class Tile
{
private:
Enchantment* m_ench;
Army* m_occupant;
ResourceType m_land_type;
Feature* m_feature;
Gate* m_gate;

public:
Tile();
Army* Occupant() const;
void Occupant(Army* occupant);

Enchantment* TileEnchantment() const;
void TileEnchantment(Enchantment* ench);

ResourceType LandType() const;
void LandType(const ResourceType& land_type);

void SetGate(Gate* new_gate);
Gate* GetGate() const;

void SetFeature(Feature* feature);
Feature* GetFeature() const;

~Tile();
};

Enchantment, Feature, Army, and Gate are all abstract base classes
(ABC's). My question is this: what is the best way to implement
accessors of those fields? According to Items 29 and 30, this sort of
implementation is out because it returns a handle to internal data
that is less accessible than itself:

Gate* Tile::GetGate() const
{
return m_gate;
}

But according to item 31, this is out too because it dereferences a
pointer initialized within a function:

Gate* Tile::GetGate() const
{
return m_gate.copy(); //Where copy returns a pointer to an object
created by new
}

What are my options? Should I return const references instead? I
suppose that I could return std::tr1::shared_ptr's, but I have to
believe that someone had a solution to this problem before that was
invented that didn't involve handles to internal data or memory leaks.

Thanks for your help.

 

[Daniel T.]

I've been reading Scott Meyer's book, "Effective C++", and I think
that I've confused myself terribly. I have a class like this:

class Tile
{
private:
Enchantment* m_ench;
Army* m_occupant;
ResourceType m_land_type;
Feature* m_feature;
Gate* m_gate;

public:
Tile();
Army* Occupant() const;
void Occupant(Army* occupant);

Enchantment* TileEnchantment() const;
void TileEnchantment(Enchantment* ench);

ResourceType LandType() const;
void LandType(const ResourceType& land_type);

void SetGate(Gate* new_gate);
Gate* GetGate() const;

void SetFeature(Feature* feature);
Feature* GetFeature() const;

~Tile();
};

Enchantment, Feature, Army, and Gate are all abstract base classes
(ABC's). My question is this: what is the best way to implement
accessors of those fields? According to Items 29 and 30, this sort of
implementation is out because it returns a handle to internal data
that is less accessible than itself:

Is the Gate object less accessible than the Tile object? What other
objects hold the Gate object?

Gate* Tile::GetGate() const
{
return m_gate;
}

The above won't even compile. Either the return has to be const or
the method can't be.

But according to item 31, this is out too because it dereferences a
pointer initialized within a function:

Gate* Tile::GetGate() const
{
return m_gate.copy(); //Where copy returns a pointer to an object
created by new
}

What are my options?

The most obvious option is to not have a "GetGate()" function. Move the
code that would use "GetGate()" into Tile.

 

[Pete Becker]

What are my options? Should I return const references instead? I
suppose that I could return std::tr1::shared_ptr's, but I have to
believe that someone had a solution to this problem before that was
invented that didn't involve handles to internal data or memory leaks.

There's internal data, and then there's internal data. An object that
aggregates other objects technically has internal data, but there's no
harm in allowing direct access to that data. Think of a vector<int>,
which lets you change the value of the nth element of the vector. On the
other hand, allowing user code to modify the pointer that points to the
vector's storage array could easily lead to disasters. That's because
the internal pointer is part of the implementation, and shouldn't be
exposed.

--

-- Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com)
Author of "The Standard C++ Library Extensions: a Tutorial and
Reference." (www.petebecker.com/tr1book)

 

[Tigera]

Is the Gate object less accessible than the Tile object? What other
objects hold the Gate object?


The above won't even compile. Either the return has to be const or
the method can't be.




The most obvious option is to not have a "GetGate()" function. Move the
code that would use "GetGate()" into Tile.

Ah, but sir, under gcc-4.1.2, it really did compile! Now, mind you I
only have it compiling at this point, and the program using this isn't
doing anything yet, so it remains to be seen if it will actually
work. I believe that you may be confusing the implementation with
something like:

const Gate* Tile::GetGate() const

In that case, you'd be right - I couldn't simply return it without a
const_cast.

Hey, what if I return a const Gate& instead? I guess that might still
give me a memory leak. My understanding from the book is that's like
std::string::c_str() is implemented.

 

[Daniel T.]

Ah, but sir, under gcc-4.1.2, it really did compile!

My mistake. It is the pointer that would have to be const, not the value
the pointer points to.

 

[Tigera]

There's internal data, and then there's internal data. An object that
aggregates other objects technically has internal data, but there's no
harm in allowing direct access to that data. Think of a vector<int>,
which lets you change the value of the nth element of the vector. On the
other hand, allowing user code to modify the pointer that points to the
vector's storage array could easily lead to disasters. That's because
the internal pointer is part of the implementation, and shouldn't be
exposed.

--

-- Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com)
Author of "The Standard C++ Library Extensions: a Tutorial and
Reference." (www.petebecker.com/tr1book)

I guess that I could make the argument that the whole point of the
Tile class (which represents an area on a map) is that it's just a
storage location for those three members. But it still doesn't get
past Item 30. As Scott Meyers put it, "your overworked, underpaid
compilers went to lots of trouble to make sure your access
restrictions aren't circumvented." After all, Tile is the "owner" of
the pointer (i.e, it's responsible for allocating and deleting the
memory), at least that's the current design.
Maybe my only option really is to move the Gate/Enchantment/Army
functions to Tile and avoid anyone ever seeing those classes at all.

 

[Pete Becker]

There's internal data, and then there's internal data. An object that
aggregates other objects technically has internal data, but there's no
harm in allowing direct access to that data. Think of a vector<int>,
which lets you change the value of the nth element of the vector. On the
other hand, allowing user code to modify the pointer that points to the
vector's storage array could easily lead to disasters. That's because
the internal pointer is part of the implementation, and shouldn't be
exposed.

[quoted sig elided]

I guess that I could make the argument that the whole point of the
Tile class (which represents an area on a map) is that it's just a
storage location for those three members. But it still doesn't get
past Item 30. As Scott Meyers put it, "your overworked, underpaid
compilers went to lots of trouble to make sure your access
restrictions aren't circumvented." After all, Tile is the "owner" of
the pointer (i.e, it's responsible for allocating and deleting the
memory), at least that's the current design.
Maybe my only option really is to move the Gate/Enchantment/Army
functions to Tile and avoid anyone ever seeing those classes at all.

As they say in the army, if the map doesn't fit the terrain, trust the
terrain.

--

-- Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com)
Author of "The Standard C++ Library Extensions: a Tutorial and
Reference." (www.petebecker.com/tr1book)

 

[Daniel T.]

I guess that I could make the argument that the whole point of the
Tile class (which represents an area on a map) is that it's just a
storage location for those three members. But it still doesn't get
past Item 30. As Scott Meyers put it, "your overworked, underpaid
compilers went to lots of trouble to make sure your access
restrictions aren't circumvented." After all, Tile is the "owner" of
the pointer (i.e, it's responsible for allocating and deleting the
memory), at least that's the current design.

I ask again, is that really the case? Do Tile objects own the objects
pointed to by the pointer, or do other objects use those objects too? It
sounds like the latter is the case.

Maybe my only option really is to move the Gate/Enchantment/Army
functions to Tile and avoid anyone ever seeing those classes at all.

That's an option, not necessarily the best. You might want to move this
thread over to comp.object. It is more a discussion of OO design than
C++ coding.