0

If I have this private map of scenes in my header file

std::map<std::string, Scene> scenes;

Should my public getter be like so?

std::map<std::string, Scene>* getScenes() { return &scenes; }

Still getting my head around pointers in c++ so apologies if it seems like a dumb question

JoelOliMclean
  • 3
  • 3
  • Assuming all of this is actually members in some unstated class, probability is high that you don't need a pointer in the first place; a reference will do, and *maybe* even a value-return, depending on unstated purpose of providing a getter in the first place. Also, consider whether `getScenes` should be `const`, because chances are it can be. Finally, consider whether the resulting reference can/should also be `const`, again, depending on the purpose of providing that getter. – WhozCraig May 30 '20 at 21:40
  • Why do you need a pointer here? Returning a plain pointer to a private member kind of defeats the whole purpose of making the member private. If you want it to be accessed by non-members it shouldn't be private in the first place. It's more likely that your getter should return something like a const reference or even a single element in the map depending on what you're doing. – eesiraed May 30 '20 at 21:43

2 Answers2

1

Using a pointer is rarely a good idea, considering you can return by reference.

std::map<std::string, Scene>& getScenes() { return scenes; }

This however is also not very good and defeats the purpose of making the member variable private, as does returning a pointer to it.

To access the member variable to view the data and not change it, which lends some sense to having a getter of a private member variable, you should return a const reference:

const std::map<std::string, Scene>& getScenes() const { return scenes; }

I'll just add that some defend that getters and setters are just clutter and you shoud just go ahead and make the member variable public if need be.

Others defend that it should be used with parsimony.

Others still, that they sholud be used but only because they're the lesser o two evils.

anastaciu
  • 23,467
  • 7
  • 28
  • 53
  • 1
    returning a non-const refrence is rarely a good idea. It compeltely defeats the purpose of making the member private. If you return a non-const reference then the getter is really just clutter and of no real use – 463035818_is_not_an_ai May 30 '20 at 23:19
  • 1
    @idclev463035818, yes, I agree, since the OP has a simple pointer to the member I started with that simple change, I added some clarity to my answer. – anastaciu May 30 '20 at 23:31
0

Unless you need to, returning a pointer to a member that isn't stored as a pointer isn't a great idea. Passing pointers around can get messy e.g. if the original object goes out of scope you'll end up with a dangling pointer. Returning by const reference should be your first port of call:

const std::map<std::string, Scene>& getScenes() const { return scenes; }

This will avoid a copy while preserving const correctness. This obviously isn't a one size fits all approach, but quite often if you want a getter for a non-primitive type, you will want to return by const reference.

castro
  • 409
  • 3
  • 13