C++ Programming :: Aggregating ABC's and pointer correctness


	Aggregating ABC's and pointer correctness 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. - Hide quoted text - Tigera <tig@rocketmail.com> wrote: > 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. Tigera wrote: > 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) On Jun 3, 2:17 pm, "Daniel T." <danie@earthlink.net> wrote: - Hide quoted text - > Tigera <tig@rocketmail.com> wrote: > > 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. 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. Tigera <tig@rocketmail.com> wrote: > > > 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. > 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. On Jun 3, 2:54 pm, Pete Becker <p@versatilecoding.com> wrote: - Hide quoted text - > Tigera wrote: > > 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) 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. - Hide quoted text - Tigera wrote: > On Jun 3, 2:54 pm, Pete Becker <p@versatilecoding.com> wrote: >> Tigera wrote: >>> 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. >[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) Tigera <tig@rocketmail.com> wrote: > 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.