Should getters and setters be avoided?

Started by
23 comments, last by JohnnyCode 7 years, 11 months ago

I have read this article and watched the webinar of Yegor and various other sources that imply that getters and setters can and should be completely avoided. While I do agree on a lot of things what Yegor says, I am still struggling to understand it completely. I do agree on having methods that do the thinking for the object itself instead of a plain setter. For example if we have a bank account we should not have setBalance(double balance) but instead withdraw(double amount) and deposit(double amount) this would clearly benefit the safety and readability of the exposed functionality for this object. So I agree that setters are in some way evil, we should always ask the object to do something and let it be handled by it's own behavior instead of pulling some data out and work with it somewhere else only to put it back in again. I also agree with him that this leads to better designed classes and easier abstraction when you want to split it up.

However, I am not sure how I would be able to get rid of getters, and I might not want to, but I am intrigued by this idea of Yegor. If we take this balance object, it seems to me it is important we can see it's properties, in this case the balance and perhaps the transactions that have been done on it. How would I do this without some sort of getter? I mean showBalance() would still function exactly like a getter. Even if I would add some sort of GUI to the balance object to display it's contents I still need to be able to "get" that GUI element. In the real world I am able to see some of the properties from objects and for some I might have to ask the objects. For example I can see the color of a box so I should be able to "get" it's color directly. I should not be able to get the combined weight directly from the box, I ask the box to calculate the weight of it's contents and itself and return this, I guess the latter does not fall under the term "getter" for Yegor.

One of the reasons Yegor mentions is the dependency of clients connected to the object. Say, we suddenly want to know who withdrawn something from the object? I am not understanding why his "idea" would help us in any way. We still have to change the withdraw method and all clients using this withdraw method need to be changed. I guess I still have no clue what he is talking about.

He also implies in his webinar that all objects should be immutable. This idea sounds crazy to me, he is actually proposing to pass a new object every time we want to mutate it.


public Balance withdraw(double amount)
{
  return new Balance(balance - amount);
}

I am aware that most people think this is nonsense but I would like to hear why. Or better, how this would help us in any way. I can only think of this as much slower and more typing: myBalance = myBalance.withdaw(...). What is the use of a object when it is not allowed to change at all, and when it wants to change it clones itself. This eventually results in long constructors since all the final properties that are allowed to mutate (yes it sounds crazy) need to be set in the constructor.

I'd like to here your thoughts about the ideas from Yegor in the article. What are your thoughts and why do you think it's good or bad?

Advertisement

One of the reasons Yegor mentions is the dependency of clients connected to the object. Say, we suddenly want to know who withdrawn something from the object? I am not understanding why his "idea" would help us in any way. We still have to change the withdraw method and all clients using this withdraw method need to be changed. I guess I still have no clue what he is talking about.

If you can only withdraw money by calling 'withdraw' you can just add a new parameter to withdraw and your compiler will scream at you until all places where money is withdrawn supply that information. If you just manually decreased amount everywhere you have to find all uses of 'amount' manually in the code, identify which are actually withdraw operations and add the withdrawer information manually (help the compiler can be to you in that case: zero).

This eventually results in long constructors since all the final properties that are allowed to mutate (yes it sounds crazy) need to be set in the constructor.

If your constructor becomes so complicated you have to pass so many parameters you probably have a class which just tries to do far too much. Look into the Single Responsibility Principle (https://en.wikipedia.org/wiki/Single_responsibility_principle). Having immutable classes where reasonably possible certainly simplifies a lot of problems, but especially in the context of game programming it is not always doable. Spending a few moment to ponder "Do I actually need that to be mutable?" for a new class is not a bad idea though.

Edit: Stupid editor screwed up the link...


He also implies in his webinar that all objects should be immutable. This idea sounds crazy to me, he is actually proposing to pass a new object every time we want to mutate it.
public Balance withdraw(double amount)
{
return new Balance(balance - amount);
}

Ouch, that's an interesting idea but surely that is just asking for trouble.

Interested in Fractals? Check out my App, Fractal Scout, free on the Google Play store.


If you can only withdraw money by calling 'withdraw' you can just add a new parameter to withdraw and your compiler will scream at you until all places where money is withdrawn supply that information. If you just manually decreased amount everywhere you have to find all uses of 'amount' manually in the code, identify which are actually withdraw operations and add the withdrawer information manually (help the compiler can be to you in that case: zero).

Yeah, I was actually putting that under the obvious and was looking for some supreme way I was not aware of. It is actually the same as just making a variable public to access it. Then you decide there needs to be done more to function. You make the variable private and create a method for it. The IDE will throw errors since the property is not accessible anymore and you have to change it to the method you created.


If your constructor becomes so complicated you have to pass so many parameters you probably have a class which just tries to do far too much. Look into the Single Responsibility Principle (https://en.wikipedia.org/wiki/Single_responsibility_principle). Having immutable classes where reasonably possible certainly simplifies a lot of problems, but especially in the context of game programming it is not always reasonably possible. Spending a few moment to ponder "Do I actually need that to be mutable?" for a new class is not a bad idea though.

Well yeah, I am doing that as much as possible. But some classes are just getting large no matter what. Since we are on gamedev take a creature class. It holds a ton of mutable properties like many stats and equipment, position, effects, etc. These all belong to that creature. I can abstract all stats to there own implementation but creature would still hold a stats object, likewise for it's inventory and equipment, etc. If we would make all these properties final then each time a player moves I have to create a new creature with all these properties in it's constructor. There is only so much to abstract unless we rip apart the creature class and map everything separately which seems ridiculous to me and would run into the same issues for the class that is implementing this.


Map<Integer, Creature> CreatureMap = new HashMap<Integer, Creature>();
Map<Integer, Stats> statsMap = new HashMap<Integer, Stats>();
Map<Integer, Equipment> equipmentMap = new HashMap<Integer, Equipment>();

public void createCreature()
{
  int index = getNextIndex();
  creatureMap.put(index, new Creature())
  statsMap.put(index, new Stats());
  equipmentMap.put(index, new Equipment);
}

public void mutateCreature(int index)
{
  creatureMap.put(index, creatureMap.get(index).mutateSomething());
}

This looks crazy to me (but is it? I'm trying to learn something here and my assumptions or thinking pattern might be completely wrong), the creature should have it's properties inside it. Some properties can be abstracted in there own classes if that makes common sense but one can still end up with an object that has many properties. If we do what Yegor suggests we do, making all these properties final then we inevitably end up with large constructors or a builder pattern with a large amount of mandatory properties.

As usual, you should not take everything too seriously.

There is a common behavioural pattern of someone doing something, then someone else noticing that it's actually a clever idea and calling it "best practice". Then someone calls it a "pattern", and soon a witch hunt starts against anyone not applying the pattern. A year later, someone says "considered evil" and the word "antipattern" comes up, and the same witch hunt starts in the opposite direction. And then, another year later, someone says "no longer considered evil".

Yes, there are people who use getters and setters in a way that makes you facepalm and moan. But there are also legitimate, sensible applications of getters and setters.

Just like with every feature/technique/pattern there are people who recommend it, sometimes for good reasons, sometimes for strawman reasons, and sometimes for religious reasons, and an equal number of people who say the exact opposite. And there are people who use it sensibly, when it's applicable. Try to be among the latter group, and you're good.

Whenever it comes to "philosophy" or "religion", it is often hard or impossible to tell what's true and what is not. There is usually no absolute either (neither absolute right nor wrong). Consequently, the examples given to justify one's religion may make perfect sense or may be utter bullshit, depending on which angle you're coming from.

For example, take the dog example on Yegor's site. You must think of a dog as a living object. You cannot change its weight, and the dog is immutable. OK, alright... but as anyone who has ever had a pet animal knows, an animal is nowhere near immutable. They grow, they grow fat, they get sick, and they die. Surely, the notion of having to euthanize the animal and replace it with a similar looking one whenever it gains weight or whenever it gets ill is not very "natural", is it?

Thinking about the dog as an object, the dog cannot return NULL, and that is a good thing. Why not, exactly? Real dogs do return NULL when they feel like it. Returning NULL, or figuratively telling "I don't want" or "I don't have any" in another way is a legitimate thing to do. Should a dog be allowed to throw exceptions? You tell me.

A bank account should be immutable? So each time you WithdrawMoney(), the bank reopens a new, identical account for you, and deducts the amount from only the original account? Well, that looks like a pattern that should be universally applied! Please tell my bank. Also tell them that I should be able to modify my account balance directly, it's so annoying that WithdrawMoney(-100000) fails with an error message. SetBalance(100000000) doesn't seem to work either.

Another scenario where getters/setters are legitimate (in the same sense of doing plausability/validity checks) that immediately comes to mind is placing things in a GUI. Let's say a widget or window has the SetSize setter, which will... duh... set its size. How useless! Except... you want things to be visible on screen, and SetSize will make sure that's the case, even if you call it with invalid values.

Another thing is, obviously, hiding implementation details. Rather than assigning true to a boolean, you might call SetEnabled or Enable (which is really the same thing). Why SetSize anyway? I can just change that integer after making sure my values are sane, right?

SetEnabled might acquire a mutex because... surprise... the implementation is multi-threaded, which you didn't know of course, and just changing the value whenever you feel like it might have desastrous consequences. Maybe you don't need a mutex, but you need an atomic operation to set a pointer to an object in order to avoid having a worker pick up an intermediate garble value. Or something, whatever it is. The Setter makes sure that whatever it is, it's what happens, and you need not know.

It also makes sure that if, for whatever reason, you decide to change your internal representation from a 32-bit integer to a 64-bit integer 10 years in the future, the existing accessors remain valid. GetTickCount under Windows (not object oriented, but same thing really) is a real-life example, which is nothing but an "accessor" that used to simply read the 32-bit integer at 0x7FFE0008. It so happens that now there also exists GetTickCount64 which does the same, only with a bigger integer, but software using the old function still works. And it works under 64-bit Windows as well where the address is a different one. Under the hood, pretty much everything has changed, but 15-year old software still works the same and doesn't need to know. And not only is it "doesn't need to know", but indeed "doesn't want to know".

SetSize in your vector-like class might just change an integer or might have to reallocate. In your database object, a function with the same name might change a number, or might cause a file's logical size or a mapping to be altered, optionally dealing with OS errors.

Methods on objects should be verbs or questions.

Sometimes it's appropriate that the verb is to set some state.

But generally no, the state of the object should not be visible or changeable from outside the object. So you don't need getters and setters.

If you find yourself with a "bag of values" object, just make it a verbless bag of objects. And make all the members public and save a bunch of typing.

One of the common flaws of developers is that given a setter interface, they try and drive the objects through it, leading to pages of boring settering code instead of a single verb operation to abstract that away -- and which, appropriately named, tells you what it does.

There's the question of whether you are going for pure, dogmatic object-orientation, or something more pragmatic. Sometimes, you have chunks of data that make sense to be grouped together, and don't really have any behavior associated with them. Go wild with getters and setters there - it's a lot more future-proof and change-friendly than relying on public fields, instead, when it becomes necessary. Even better if you're using a programming language that has support for C#-style properties that sugar over the getter-setter boilerplate.

Especially if you are trying to program in a more functional style, you end up wanting lots of more behavior-poor data objects with public getters, though you would tend towards keeping your setters private, as leaning harder towards immutability tends to be the better choice.

Eric Richards

SlimDX tutorials - http://www.richardssoftware.net/

Twitter - @EricRichards22

Like Samoth said, while in many (or in my experience, most) cases it results in bloat and overengineering, using getters and setters is, in some cases, necessary. I personally use them only ever so occasionally when I am absolutely sure that is the type of encapsulation I need. Basic getters and setters do not provide thread safety, they do not result in cleaner code and in my perspective they do not provide too much safety in the way of accidentally writing into a raw member variable. Unless... you have a reason to classify access to these member variables, that is.

A somewhat more interesting scenario is providing a seaparate accessor structure, which then friends only those classes that should have access to your base structure. Eg:


struct base {
   private:
     friend struct base_accessor;

     int a;
};

struct base_accessor {
    private:
      friend struct interface1;
      friend struct interface2;

     public:
       inline void set_a(base& b, int a) const { b.a = a; }
       inline int get_a(base& b) const { return b.a; }
}

Now only interface1 and interface2 have access to base::a and need to do so via an instance of base_accessor. My engine codes uses this to limit certain components from accidentally writing into a class to limit accessibility to a given thread. Also, this way you can separate the write interface from the read interface by defining a base_writer and a base_reader, should you need to.

Also, getters and setters are invaluable when you're maintaining an intermediate variable, which might be updated only when one of its component variables changes. If you do use them on raw variables, you should, in the very least, declare them inline and make the getter const so you'll have const correctness.

As usual, you should not take everything too seriously.

There is a common behavioural pattern of someone doing something, then someone else noticing that it's actually a clever idea and calling it "best practice". Then someone calls it a "pattern", and soon a witch hunt starts against anyone not applying the pattern. A year later, someone says "considered evil" and the word "antipattern" comes up, and the same witch hunt starts in the opposite direction. And then, another year later, someone says "no longer considered evil".

I can't upvote this enough ; )

The decision on having accessors and mutators (also called 'getters' and 'setters') should come down to the nature of the object. Remember the Single Responsibility Principle.

If the single responsibility is a holder for data, where the purpose is to store and retrieve through controlled access, then using accessors and mutators fits the responsibility. The use of the class is typically through the nouns, the data it contains.

If the single responsibility is to store and hold data where uncontrolled access is better, then direct access better fits the responsibility. Don't introduce the potential overhead of acccessors and mutators if there is no need.

If the single responsibility is to take actions, which is the typical pattern, then usually the implementation is done through verbs. Interfaces should be collections of tasks as verbs. This allows you to abstract away the implementation details and child classes can take different actions to complete the tasks if they need.

When it makes sense to operate as verbs, do that. When it makes sense to operate as nouns, do that. When it doesn't make sense, don't.

If you've got a class with 30 functions that do nothing but set and get variables and do no other work, provide no access control or validation or restrictions, you're probably got no need for the accessors and mutators. Make it a simple structure.

If you've got a class that every time you use it you do a test, then use the value, then do something more, such as lock-use-unlock, you've got a good indication you need controlled access. Accessors and mutators are the way to implement the control structures, to verify that locks are in place before use or otherwise do whatever validation you need..

If you've got a class where you are manually manipulating all the inner pieces, where you are manipulating several pieces of data rather than just one, where you seldom act on individual pieces but instead typically act on many pieces of data and keep them in sync together, then you probably should not be touching the individual pieces of data but instead rely on functions that are verbs to do the actions.

This topic is closed to new replies.

Advertisement