C++ Getter/Setter (Alternatives?)

Question

Okay, just about everywhere I read, I read that getters/setters are "evil". Now, as a programmer who uses getters/setters often in PHP / C#, I do not see how they are alive. I have read that they break encapsulation, etc etc, however, here is a simple example.

class Armor{

  int armorValue;
public:
  Armor();
  Armor(int); //int here represents armor value
  int GetArmorValue();
  void SetArmorValue(int);
};

Now, lets say getters and setters are "evil". How are you supposed to change a member variable after initialization.

Example:

Armor arm=Armor(128); //armor with 128 armor value

//for some reason I would like to change this armor value
arm.SetArmorValue(55); //if i do not use getters / setters how is this possible?

Lets say the above is not okay, for whatever reason. What if my game restricts armor values from 1 to 500. (No armor can have a piece that has more than 500 armor or less than 1 armor).

Now my implementation becomes

void Armor::SetArmor(int tArmValue){
      if (tArmValue>=1 && tArmValue<=500)
         armorValue=tArmValue;
      else
         armorValue=1;
}

So, how else would I impose this restriction without using getters/setters? How else would I modify a property without using getters/setters? Should armorValue just be a public member variable in case 1, and the getters/setters used in case 2?

Curious. THanks guys

Answer 1

You have misunderstood something. Not using getters/setters breaks encapsulation and exposes implementation details, and can be considered "evil" for some definition of evil.

I guess they can be considered evil in the sense, that without proper IDE/editor support, they are somewhat tediois to write in C++...

One pitfall of C++ is to create non-const reference getter, which allows also modification. That's same as returning a pointer to internal data, and will lock that part of internal implementation, and is really no better than making field public.

Edit: based on comments and other answers, what you heard probably refers to always creating non-private getter and setter for every field. But I would not call that evil either, just stupid ;-)

Answer 2

Being slightly contrarian: yes, getters and setters (aka accessors and mutators) are mostly evil.

The evil here is not, IMO, so much from "breaking encapsulation", as from simply defining a variable to be of one type (e.g., int) when it's really not that type at all. Looking at your example, you're calling Armor an int, but it's really not. While it's undoubtedly an integer, it's certainly not an int, which (among other things) defines a range. While your type is an integer, it's never intended to support the same range as an int at all. If you want Armor to be of a type integer from 1 to 500, define a type to represent that directly, and define Armor as an instance of that type. In this case, since the invariant you want to enforce is defined as part of the type itself, you don't need to tack a setter onto it to try to enforce it.

template <class T, class less=std::less<T> >
class bounded {
    const T lower_, upper_;
    T val_;

    bool check(T const &value) {
        return less()(value, lower_) || less()(upper_, value);
    }

    void assign(T const &value) {
        if (check(value))
            throw std::domain_error("Out of Range");
        val_ = value;
    }

public:
    bounded(T const &lower, T const &upper) 
        : lower_(lower), upper_(upper) {}

    bounded(bounded const &init) 
        : lower_(init.lower), upper_(init.upper), val_(init.val_)
    { }

    bounded &operator=(T const &v) { assign(v);  return *this; }

    operator T() const { return val_; }

    friend std::istream &operator>>(std::istream &is, bounded &b) {
        T temp;
        is >> temp;

        if (b.check(temp))
            is.setstate(std::ios::failbit);
        else
            b.val_ = temp;
        return is;
    }
};

With this in place, defining some armor with a range of 1..500 becomes utterly trivial:

bounded<int> armor(1, 500);

Depending on the situation, you might prefer to define (for example) a saturating type where attempting to assign an out of range value is fine, but the value that actually is assigned will simply be the nearest value that is within range.

saturating<int> armor(1, 500);

armor = 1000;

std::cout << armor;  // prints "500"

Of course, what I've given above is also a bit bare-bones. For your armor type, it would probably be convenient to support -= (and possibly +=) so an attack would end up something like x.armor -= 10;.

Bottom line: the (or at least "one") major problem with getters and setters is that they usually point to your having defined a variable as being of one type when you really want some other type that happened to be sort of similar in a few ways.

Now, it's true that some languages (e.g., Java) fail to provide the programmer with the tools necessary to write code like that. Here I'm trusting your use of the C++ tag to indicate that you really do want to write C++ though. C++ does provide you with the necessary tools, and (at least IMO) your code will be better off for your making good use of the tools it provides so your type enforces the required semantic constraints while still using clean, natural, readable syntax.

Answer 3

In short: they aren't evil.

It's nothing wrong with them as long as they don't leak out the internal representation. I see no problems here.