Class design: allowing users of a class to change the content of an internal container

[t36]

Hi,

Apologies if this has been asked before, I tried to do a search but I don't know quite which keywords to use for this.

I have to design a class which reads a list of persons and their details from a file, and give the ability to users to this class to change the details a save the modified list to file.

I implemented it using a map, e.g. std::map<std::string, Person>. This allows my users to quickly retrieve the Person objects via the name of the Person. However, I am doubting between to options to allow my users to modify the details of a Person object.

Should I choose option A:
Person &mutablePerson(const std::string &name);

or option B:
void setPerson(std::string &name, const Person &newDetails).

Option A seems more efficient as it avoids two copies, however it also seems to break encapsulation.
Option B, while "safer" will use more copies.

Curious about your thoughts on this.

Greetings,

t36

[shadowmouse]

Option B seems better, or even better simply implement void setSpecificDetail(new Detail) functions as this is one of the primary rules of encapsulation.

[Duthomhas]

Unless your container class has specific restrictions on its content (such as being immutable) then both options are fine and do not break encapsulation.

Standard caveats pertaining to references apply.

Personally, I don't think container classes have any business modifying access contracts with the contained items.

[t36]

Hi,

Thanks for your answers, they have helped a lot :). The standard caveats you talk about, do you refer to the fact that the reference might no longer be valid if the size of the container changes?

Greetings,

t36

[Duthomhas]

Yes, etc. There are other ways to make references invalid as well, but you've typically got to work at it.

[JLBorges]

If the class person has a (mutable) property name, then
Person &mutablePerson(const std::string &name);
would lead to a logical inconsistency if the user modifies the name of the person.

const Person& the_person( const std::string &name );
is fine; references, iterators and pointers to elements in a std::map<> are not invalidated by insertions or deletions of other elements. (We would need to throw an exception if the name is not found; use std::std::map<>::at()).