Accessors/Mutators, yay! Or...?

[Creature]

Hello everyone

Recently I've stumbled upon a bit of an impasse. Usually, people tend to recommend the use of accessors and mutators for classes because they provide data encapsulation or hiding. This is all good and well for my Foo class here:

Code: Select all

class Foo
{
private:
   int SomeVar;

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

In this case, the user can't directly access SomeVar because it's private, he or she can only use the mutator/accessor or the getter/setter. But my impasse is what happens when I have the following situation:

Code: Select all

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; }
};

There are a couple of potential 'issues' for me that arise here:

• 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?

The iteration and accessing of the container is also totally obfuscated; each time I want to iterate I have to do something like:

Code: Select all

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

Which I can avoid by creating a variable to temporarily store the reference in, like so:

Code: Select all

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

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

But this all seems very obfuscated and over the top for me. Being the good *cough* programmer that I am, I've always done it this way, thinking that I'd better just do it and forget it. I guess my question is: should one always use mutators/accessors (or: when should one use them)? Is it just me that's annoyed by this and will I just have to learn to live with it?

[bewing]

OOP languages encourage (and sometimes force) you to do inefficient things. If you care about efficiency and speed you should be using a different language. Yes, what you describe is very much the standard way to do things in C++, yes it sucks, yes it's inefficient, yes it annoys the hell out of me too -- and that's why I write my code in C and ASM with lots of lovely global variables. Global variables are part of the intrinsic design for how the silicon actually works, and avoiding globals for "aesthetic reasons" is damned foolishness IMNSHO.

[Creature]

I was kinda hoping for an excuse to use more efficient (but less OO) constructs in (more or less) OO languages like C++, but from what's becoming clear to me, it doesn't seem I have much of a choice. It actually surprises me that this has become more or less of a 'standard' in today's programming whilst it's so inefficient and it's completely useless (unless of course, the mutator or accessor does more than just setting/getting the respective variable).

I know languages like as C# solve the problem by having getters and setters embedded into the language itself. You can actually write getters and setters and C# won't actually treat them as functions (you access the variable like it's a public variable, but you're actually indirectly passing through the respective getter or setter). In Java, not using them is probably considered heresy, whilst C++ is kind of caught 'in between': you can do it the Java-way, but it's not very attractive, or you can *want* to do it the C#-way, but that's not an option (unless you use some hacky constructs, which are totally the opposite of getting readable and proper code).

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

It's not that I don't want to use public variables, they look much cleaner IMHO if the getter/setter doesn't do anything else than get/set the variable. But I fear it wouldn't be appreciated if you were to use it in a more 'productive' environment (e.g. in the workplace), where it could be considered a 'bad habit'.

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; });

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) ...
}

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

In which you have a massive amount of calls to the accessor. It's not that I don't like iterators (not at all, I love algorithms and iterators), but it's just the accessor that bothers me.

[NickJohnson]

I think it all boils down to one fundamental tradeoff that most languages make: simplicity versus safety. The whole "data hiding" idea assumes that any programmer that comes along and tries to use a class will probably screw it up somehow, so that doing checks on accesses and modifications of class data is a necessary measure, even though it has a complexity and performance penalty. For large, complex systems developed by average programmers, this makes a lot of sense. Of course, here at osdev a lot of people prefer very powerful, unchecked languages (C, ASM), because they are developing relatively simple systems and are more skilled. This tactic tends to break down, however, as projects get larger (what's the largest program ever written in assembly?)

[Solar]

*Sigh*

I cannot count how many times I've been in this particular discussion. Usually, I find that it is led on the wrong level - the one of syntax. The underlying problem here is a semantic one.

Code: Select all

class Bar
{
    private:
       Foo x;
};

The underlying question is, why should you grant access to x anyway?

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.

The only case where the question of accessors / mutators should actually arise is if you're trying to write a collection (struct) in a pure-OO language (like Java). In that case, you're violating pure-OO already, so you shouldn't worry about the "right" way to write getters / setters for your class members, but about why you're needing a collection (struct) in the first place - because your design is not pure OO already, and there is no "right" way to write getters / setters in that particular country.

I don't think I have to elaborate on what I think about Java IDE's that create getters / setters automatically (or Java programmers that use them).

As for the mentions of OO being inefficient et al., that's bollocks, especially when talking about C++. One, C++ doesn't require, or even encourage, to write "pure" OO, it openly encourages multi-paradigm programming. If you need a struct, use a struct, and f*** the "pure" OO evangelists. Two, even a "pure" OO design can be efficient, but if you're worrying about accessors and mutators, you're not doing OO design, period, and don't blame the paradigm for it.

[Creature]

I realize the discussion is fairly popular on the internet (I remember being totally confused when I only just started programming), however the discussion is usually ends up with one side being totally pro to the approach ("You MUST use them") and the other side being totally contra. I'm not necessarily against or pro the approach, I'm more wondering when it should be useful.

If I read your answer correctly, you suggest that because the fact that I'm doubting my approach, I'm not doing OO design (or rather "thinking the OO way"). I guess you have a good point. I remember I used to use "struct" all my data structures (and almost never "class") just to have an excuse to use public data members. Recently I've been learning Java (I don't like it, but I have little choice) where data encapsulation is almost a must. Thinking that this was the better approach, I started writing all my C++ classes the same way, completely forgetting the fact that C++ isn't a completely OO language and I still have a choice.

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.

Suppose in this piece of code:

Code: Select all

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; }
};

... that Bar would require 'SomeVector' to do part of its job. 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 (I'm starting to think my design is seriously flawed)? One idea that comes to mind is to try and let the class itself handle the iteration, which would hover 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.

[Solar]

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?

Ask yourself why does the client code need access to the vector. What are you trying to do? This cannot be answered in the abstract, with class Bar and SomeVector.

Let's say you have a class SchoolClass, which contains a vector of Student objects. What you don't need is a operator[](), because it's stupid to access SchoolClass[17] (because a SchoolClass is, by nature, not ordered). What you also don't need is getStudentVector(), because it wouldn't be much smarter.

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

Consider what happens to your client code when you decide, later on, to replace the vector<Student> with a map<String, Student>, and the client code (which might be written by a customer company that's touchy about change) still calling getStudentVector().

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.

You're worrying about syntax again. Yes, having SchoolClass::iterator forward to SchoolClass::m_StudentVector::iterator does syntactically expose the SomeVector variable in a way. But you could just as well syntactically replace m_StudentVector with m_StudentMap (or any other STL-compliant container) without having to change ugly "...vector" references in client code. And you're doing the semantically correct thing, that is iterating over a SchoolClass, instead of getting a StudentVector from a SchoolClass and iterating over that. (The latter is C thinking translated 1:1 to C++...)

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.

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]

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?

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.

Bar doesn't inherit from a container class, so how can you call the begin() and end() functions on BarObj (which is an object of the Bar class)? It's the container Bar::SomeVector that has begin() and end() member functions, not BarObj itself. Am I missing something here or did you refer to the fact that they should be implemented in Bar itself?

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

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.

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.

I guess Java slightly 'poisoned' my way of thinking and caused me to forget my roots in C++ .

Thanks everyone, your answers have been most helpful (of course more responses are always appreciated).

[Solar]

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.

Slightly brain-dead, isn't it? Keeping boys and girls in seperate vectors in a SchoolClass object that obviously represents a mixed class?

That's a problem with OO discussions like this. Too often, people are talking in the abstract, looking for cut & dried, hard & fast rules which could be applied like templates to whatever situation might arise. But OO doesn't really work that way: The example doesn't make sense, so any solution I could offer will make even less sense.

I could suggest having a SchoolClass::boys_iterator and a SchoolClass::girls_iterator. Or I could suggest having a SchoolClass::get_iterator( StudentGender gender ). Or I could suggest reworking your design so that the problem doesn't even show up. But since the example is so bad to begin with, none of this would "ring true".

Again, keep in mind that proper OO design is about semantics just as much (if not more) than syntax.

[Creature]

You are right, the example doesn't make sense, but I was merely trying to make a point using the example you provided. There are indeed several solutions to the problem (of which you summed up some), including some of the ones you listed in the event that a class had multiple containers. I guess I was looking for the "cleanest" way to do it (or better: the way good programmers do it or as one could best do it).

Your advice has given me some material to think about, thanks Solar.

[qw]

Just a thought. I can imagine that mutators may be useful when setting a member must have an additional effect, for example:

Code: Select all

class some_silly_implementation_of_a_dynamic_array
{
private:
    void* ptr;
    size_t size;

public:
    void set_size(size_t x)
    {
        ptr = realloc(ptr, x);
        size = x;
    }
};

I am no C++ expert at all so I'll let Solar shoot at this

[Combuster]

Of course you would want to use STL for dynamic arrays making this a bad example, but setters/getters are the typical place to do stuff besides changing a member variable, like updating or invalidating caches and sanitizing input.

Try the following (more realistic) case where you are packaging objects for shipping and you want to maintain a current total to avoid summing all objects on each request:

Code: Select all

Object::setWeight(int grams)
{
    int difference = grams - this->weight;
    this->weight = grams;
    if (parent_container) parent_container->addWeight(difference);
}

[Solar]

Both of which being rather bad examples, of course. (Sorry.)

@Hobbes: You don't set the size of an array. You don't set a member, period. Well, sometimes you do, but generally it's a sign that you're looking at a bad design, see above. Client code simply has no business poking the members of an object.

@Combuster: The object has no business sending a message to a parent container (or checking for a parent container in the first line). The object should be completely constructed (including weight - ideally by the constructor) before being added to the parent container. The running total should be updated by the add_object() (or whatever) function of the container, calling size() on the added object-



OOD is best done in a relaxed environment, ideally over a chilled bottle of beer.

[qw]

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

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

[Solar]

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

I don't do that modern tech-gizmo stuff.