OSDev.org

class Foo
{
private:
   int SomeVar;

public:
   int GetSomeVar() const { return SomeVar; }
   void SetSomeVar(int val) { SomeVar = val; }
};

class Bar
{
private:
   std::vector<int> SomeVector;

public:
   const std::vector<int>& GetSomeVector() const { return SomeVector; }
   void SetSomeVector(const std::vector<int>& vec) { SomeVector = vec; }
};

• The mutator/accessor combination seems very inefficient, since modifying the vector requires a complete copy from the getter, for it to be copied again after it was modified when the setter is called.
• The use of accessors/mutators is usually recommended, but somehow using them doesn't seem to make sense here since the mutators/accessors do nothing more than modify the internal variable. My point: why not just lend the user global access altogether?

for(int i(0); i < BarObj.GetSomeVector().size(); ++i)
{
   BarObj.GetSomeVector()[i] ...
}

const std::vector<int>& vec = BarObj.GetSomeVector();

for(int i(0); i < vec.size(); ++i)
{
   vec[i] ...
}

Berkus wrote: Yes, you can do that. You should realise what you are doing and why.

Berkus wrote: for (auto it = BarObj.begin(); it != BarObj.end(); ++it)
{
(*it) ...
}

// You'll get for free:
std::for_each(BarObj.begin(), BarObj.end(), [](VecElem e){ cout << e; });

for (auto it = BarObj.GetSomeVector().begin(); it != BarObj.GetSomeVector().end(); ++it)
{
     (*it) ...
}

// You'll get for free:
std::for_each(BarObj.GetSomeVector().begin(), BarObj.GetSomeVector().end(), [](VecElem e){ cout << e; });

class Bar
{
    private:
       Foo x;
};

• Composition. Bar needs Foo to do some certain part of its job. In this case, there should be no need (or, indeed, no way) for client code to have access (or even knowledge) of Foo.
• Subsumption. Bar is Foo plus some added stuff. In this case (ideally), Bar should be derived from Foo instead of having and exposing a Foo. (Not always possible because of limitations of the language - i.e., C++ std::string cannot be derived from.) In case of a vector, Bar should export all the necessary features (Bar::size(), Bar::operator[](), iterators, yadda yadda). But again, there should be no need to expose Foo to client code. It's a Bar object, not an object you could retrieve a Foo object from.
• Collection. Bar contains a Foo, plus a couple of other data objects, but doesn't offer any (significant) operations on them other than creation of the object, getting / setting them, and destroying the object. In this case, Bar isn't a class at all, it's a struct, and should be declared as such. Members of a struct are public by default, and there's nothing wrong with accessing them directly. We're not Java here (where everything must be hammered into the pure OOP mold), we're C++, and C++ is multi-paradigm.

Solar wrote: The class Bar could be related to Foo in three different ways:

Composition. Bar needs Foo to do some certain part of its job. In this case, there should be no need (or, indeed, no way) for client code to have access (or even knowledge) of Foo.
Subsumption. Bar is Foo plus some added stuff. In this case (ideally), Bar should be derived from Foo instead of having and exposing a Foo. (Not always possible because of limitations of the language - i.e., C++ std::string cannot be derived from.) In case of a vector, Bar should export all the necessary features (Bar::size(), Bar::operator[](), iterators, yadda yadda). But again, there should be no need to expose Foo to client code. It's a Bar object, not an object you could retrieve a Foo object from.
Collection. Bar contains a Foo, plus a couple of other data objects, but doesn't offer any (significant) operations on them other than creation of the object, getting / setting them, and destroying the object. In this case, Bar isn't a class at all, it's a struct, and should be declared as such. Members of a struct are public by default, and there's nothing wrong with accessing them directly. We're not Java here (where everything must be hammered into the pure OOP mold), we're C++, and C++ is multi-paradigm.

class Bar
{
private:
   std::vector<int> SomeVector;

public:
   const std::vector<int>& GetSomeVector() const { return SomeVector; }
   void SetSomeVector(const std::vector<int>& vec) { SomeVector = vec; }
};

Creature wrote:This would make the relation a 'composition'. In your response, you state that there should then be no need to reveal this variable to client code, which seems reasonable. In my personal project however, I have a similar case where code making use of objects of 'Bar' need access to the vector (for e.g. iterating it, reading data from the elements inside the container). Do you have any suggestions on how I could better solve this?

One idea that comes to mind is to try and let the class itself handle the iteration, which would indirectly expose the SomeVector variable to client code.

So I guess what it all boils down to is that I just have to use a struct with public variables where I see fit instead of turning them all into data-encapsulated classes where it's not necessary.

berkus wrote:

Creature wrote: BarObj is actually an object of the bar class, so in my example, it would be more like this:

Code: Select all

for (auto it = BarObj.GetSomeVector().begin(); it != BarObj.GetSomeVector().end(); ++it)
{
     (*it) ...
}

No. Why would it be that way?

Creature wrote:In which you have a massive amount of calls to the accessor.

I don't see this happening. You clearly don't understand how iterators work.

Solar wrote: Consider SchoolClass::getStudentVector()::iterator vs. SchoolClass::iterator().
Consider SchoolClass::getStudentVector()::size() vs. SchoolClass::size().

Solar wrote:If your "seeing fit" and "considering necessary" stands up to scrutiny, that is indeed the C++ way to doing things effectively. In Java, you're screwed of course.

Creature wrote:What you say makes a lot of sense, it would be much more logical to just make the SchoolClass class implement the iterator and size functions instead of having to access the vector first. However, this may pose a problem in a situation where SchoolClass has two vectors: e.g. one for girls and one for boys, in which case size could plainly return the sum of their sizes, but iterator would need to be altered or require an alternative.

Solar wrote:You don't set the size of an array. You don't set a member, period.

Hobbes wrote:When I wrote "let Solar shoot at this" I meant with a small hand gun, not with a cruise missile...