Sign in to follow this  
menyo

Should getters and setters be avoided?

Recommended Posts

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?

Share this post


Link to post
Share on other sites

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

Edited by BitMaster

Share this post


Link to post
Share on other sites


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.

Share this post


Link to post
Share on other sites

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.

Edited by menyo

Share this post


Link to post
Share on other sites

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.

Share this post


Link to post
Share on other sites

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.

 

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites

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.

 
This looks to me like an entity-component system. We've been getting lots of questions about those recently. There are some advantages to this kind of approach as long as you don't follow it too dogmatically. In particular, it lets you not bother creating the components that don't need to exist for an object - do trees really need pathfinding, for example? Does a player character really need an AI state? What about creatures that can't (by design) carry equipment? Though, this particular point applies to composition generally.
 
I'd hardly call it crazy. My own side projects follow this sort of architecture because it makes it easier to add new features and because it allows me to use existence-based processing on the resulting components. In a lot of cases (not all) this makes the resulting code simpler. Edited by Oberon_Command

Share this post


Link to post
Share on other sites

I abhor functions literally called 'getX' 'setX' but often people really mean properties when they say get/set and properties are quite useful with or without syntactic-sugar built into the language to explicitly support them.

The references article seems to be advocating for immutability which is ... an interesting concept out there but it doesn't really have a place with imperative languages.  
You'd be better suited to using LISP than C++ to explore immutable designs and there's probably something tailored for immutability out there by now - this idea is at least 20 years old and probably 40 or 50. (If your design is based on immutable objects it becomes possible to prove its correctness.)

 

Reading over the article I don't really see a strong argument - not stronger than the weak ones they list.
What is the real difference between setBall/getBall and GiveBall/TakeBall ?

 

 


We're not getting her name. We're asking her to tell us her name. See the difference?

Um ... nope. Get is a just a sloppy word that has a lot of definitions and in-context it means the same thing.

 

 

The distinction is that a property/accessors/get-set should not perform a lot of work and should not affect another object.  
They are free to do things to maintain the internal consistency of the object or do light work to get you the value you want.  

e.g. If I have a class that represents a measurement (a value and tolerance) I will probably store the value in SI units and have properties that return the values in other units.

Edited by Shannon Barber

Share this post


Link to post
Share on other sites

The distinction is that a property/accessors/get-set should not perform a lot of work and should not affect another object.  

 

Agreed. I explicitly use different words if the function has to do more work.

 

An example is 'Find'. If a search has to be done, then it's 'FindX()' instead of 'GetX()'.

 

If something potentially has to be loaded/created/processed before being returned, then I call it 'RetrieveX()'.

For example, "RetrieveAnimation()" returns a fly-weight shared animation handle, but if the animation hasn't ever been loaded (or was freed earlier because of no sharers) then the function has to actually load the animation from disk first (or else return a dummy, pop an async task, and later swap the dummy for the real thing).

 

Basically, I only use 'Get' as a word if the class already knows exactly where the variable is, and can just return a reference or const reference to it.

 

Being consistent with your naming scheme is important. I can see how someone might get burned by using 'Get()' to mean virtually anything, and then try to "fix" the problem by radically redesigning their class designs rather than just learning better function naming. Programmers often like to overengineer complex solutions to simple problems. I know I do!  :P

 

That said, immutable objects are very useful in some contexts, so I don't want to throw out that tool either. And tiny structs that have all their data members exposed publicly are also useful, and shouldn't be wantonly banned because of misuse. Fix the misuse, don't ban the tool. And if one particular person has continued difficulty learning which end of the drill to hold, then you might need to ban the drill for that one person so they don't hurt themselves or others.

Share this post


Link to post
Share on other sites

Simple getters are ok. One object might need information stored in another object for its own logic. As a common example, the UI for a shop in an RPG might want to display the player's current amount of gold and provide visual comparisons of equipment stats, so it might need to access quite a bit of information from the player class, but it [b]should not[/b] be able to modify anything directly. But usually, when you need to get one piece of data, you need to get several pieces of data - a multi-getter (one that copies related data into a struct) might make more sense than having several individual getters.

Simple setters are generally pointless. If outside logic should be able to directly set an object's members, those members should generally be public, or the class that contains that outside logic should be friends with the class holding the data, or maybe the two classes are not logically separate at all and should be combined. I really can't think of a case where a simple setter isn't a sign of bad planning, or worse, an anemic domain model.

Complex setters - setters that have logic such as access control attached, but still serve the purpose of just assigning a member to some provided value - may sometimes be necessary, but tend to be a sign of poor design.

Share this post


Link to post
Share on other sites

Simple setters are generally pointless. If outside logic should be able to directly set an object's members, those members should generally be public,


The guidelines are actually opposed to both (avoid trivial get/set and avoid public class members), which is a bit of a sore point for me, but even the discussion around the guidelines was peppered with people saying, "It's a guideline, not a rule."

Honestly, it's nice to have a super well-formed system, but you can spend years on every trivial item and still have issues. I think it's really down to experience and learning what things save time in the long run and what things don't, and what's true for you and yours may not be the same for other people.

Share this post


Link to post
Share on other sites
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.

To me, Yegor mostly seems to argue a more precise naming of you getter. As the servant says, Find, and Retrieve. I often use Compute.

 

 

 

I started in C++, and then learned Python.

 

Shock! Horror!  no private! no protected! everything is public! How can you program in this??!?

 

After a while, I found you can program in it, and it does work. I also got to understand the rationale behind it, and it's a nice one. The idea is that all objects together are trying accomplish a goal. It's not in their interest to mangle the state of other objects. I think this is true. When you write an application you never write code that knowingly destroys crucial data of another object. In your code you put down the responsibilities and each object does its share, in a co-operative fashion.

 

Everything is freely available, you can take what you need, just respect the owner.

 

From this world-perspective, a Java class looks like a house with big walls, and a high fence with electric wires, and soldiers with machine guns and guard dogs. At the gate, you can submit a request, and after approval, you get the thing you asked for.

 

Right next door, you see the same thing, a whole street filled with big fences. Together, we will achieve the goal, no?.

 

 

Luckily, in many cases, security means nothing. This is what is called trivial getter/setters:

private:
   Foo data;

public:
  Foo GiveData() { return data; }
  void BringData(Foo new_data) { data = new_data; }

Standard example of getter/setter.

 

Now, ask yourself, what makes this better than

public:
  Foo data;

The setter and getter code wraps a function around retrieval and putting, but it still allows the caller to do anything it likes.

 

This is what Yegor is fixing with making a new object. He doesn't trust you, so he make a copy of the data, to preserve his own state, no matter what. "We friends, achieve big goals, ok?" and he hands you a piece of paper, still warm from the copier.

 

 

In these cases, I just use a public variable, and happily live in Python-land.

 

 

 

 

"but it might need protection later!"

True, but that's future coding, ie prepare for things that may never happen (and within an application, also really never happen in 99% of the cases, in my experience).

Nonetheless, if the need arises, the solution is simple, since you have all source code available:

 

1. Move variable to private

2. Add getters and setters

3. Let the compiler point out where the problems are

4. Fold a getter or setter around its use.

 

 

The only case that I know where this system breaks, is in the public API of a library or sub-system that you want to change internally without breaking the code of its users.

The number of getters and setters is quite small there probably, but here it pays anyway to be more careful with decisions.

Edited by Alberth

Share this post


Link to post
Share on other sites

Luckily, in many cases, security means nothing. This is what is called trivial getter/setters:
private:
   Foo data;

public:
  Foo GiveData() { return data; }
  void BringData(Foo new_data) { data = new_data; }
Standard example of getter/setter.
 
Now, ask yourself, what makes this better than
public:
  Foo data;
The setter and getter code wraps a function around retrieval and putting, but it still allows the caller to do anything it likes.

 

First, I basically can't say it better than this.. but here is my version of this sentiment.

 

I'm of the opinion they should be avoided like the plague. I design everything such that if you need to manually access data, to read or write it is either public in an otherwise private class, or better yet in a POD structure. Most of my classes that require you to set data do so either with friend classes or a constructor. Yea friends, they can touch each others privates but once they do they're a couple.. which I am totally okay with, but I am sure some would not like this unnecessary coupling. So to each their own style.

Edited by coope

Share this post


Link to post
Share on other sites

 

Simple setters are generally pointless. If outside logic should be able to directly set an object's members, those members should generally be public,


The guidelines are actually opposed to both (avoid trivial get/set and avoid public class members), which is a bit of a sore point for me, but even the discussion around the guidelines was peppered with people saying, "It's a guideline, not a rule."

Honestly, it's nice to have a super well-formed system, but you can spend years on every trivial item and still have issues. I think it's really down to experience and learning what things save time in the long run and what things don't, and what's true for you and yours may not be the same for other people.

 

I agree that the "no class should have a member that can be set directly by a non-friend class" notion is far from universally applicable. I think it's a guideline intended to steer people away from procedural patterns without them having to know what one might look like.

Share this post


Link to post
Share on other sites

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.

 

I am a huge fan of immutability. Yes it does mean building a new structure when mutating the data but this is not as bad/costly as you might think given the other benefits you gain.

 

Take the new C# compiler Roslyn. The old compiler was written in C++ while Roslyn is C# with everything immutable. Conventional thinking would be that C++ would be faster than a .NET language but even with the object churn the new compiler is far far faster.

 

Immutability is about getting your object structure correct, your structures are more like records from F# instead of more normal OO objects. If an object is immutable you never need to clone it as you can just reference it also there are no thread based locking issues issues ever etc. When you rebuild the structure you can reuse any referenced objects so you are only rebuilding what has changed if your design is right. There are a raft of immutable collections to ease working with collections of data, just updating the minimum of data.

 

If you also pick up concepts like lenses from Haskell the update of nested data is also very easy as it simulates get/set properties rebuilding all the object graph that needs it.

 

Languages such as Haskell are actually constructed around immutable state, you require massive hoop jumping to allow mutation.

Share this post


Link to post
Share on other sites

In my opinion, getters and setters are fine. They allow you to separate data layout from access syntax. That way you can change whether a getter and setter are trivial member accesses or not without having to find all uses of a member object. Some languages have "object properties", which allow you to have methods that have the same interface as data member access. In these languages getters and setters are uncecessarily. Java and C++ aren't those languages, so getters and setters are useful despite their verbosity.

 

For example, Imagine you have a complex number class. Complex number can be stored as real and imaginary parts, or as phase and angle. Depending of how they are used, either storage method may perform better. Having getters and setters for all four of these properties would allow you to change data you store without changing all uses of your class; you can easily benchmark which is better for your application. More generally, it allows you to easily change which object properties are calculated on the fly, and which ones are stored or cached, after your first performance benchmark, instead of just when you're first designing a class.

Share this post


Link to post
Share on other sites

You need to take what you read with a grain of salt and evaluate whether it makes sense or applies to your project. ESPECIALLY when someone starts calling something always evil like in one of the linked URLs in the original post. At this point, I don't think there is anything that is always evil. I think I actually even have a good case for a goto in my rendering engine's code, which is the pinnacle of the NO-NOes in programming. (And I'm still looking to get rid of it.) So if you tell me that something as ubiquitous and proven as getters/setters are always evil, no, just no. Misused? Yeah, I think people often write a lot more getters and setters than they actually need to. But always evil? No. (And only a Sith deals in absolutes. :P)

 

The truth is that there are a lot of people in the programming circles that would like to make themselves a name by calling well accepted practices "evil" just because it's bad by some completely unpragmatic point of view, or because once upon a time someone misused the pattern or principle. Now it's get/set, last week singletons were evil. Next week if someone calls having a main() function an antipattern... well, remember I called it first here. :P

 

Don't ignore what you read, just have critical thinking. If you're a seasoned programmer, you know more about how to go with your project than some blogger with a book so listening advices and new method can never hurt, but in the end you're the one who can judge how to go with your project. A lot of those people writing about programming practices are academic Java programmers looking to sell their books or conferences. Probably the same type of people that state completely obvious things, wrap them in silly catchy acronyms like KISS, GRASP and SOLID and write PhD thesis around those, but I disgress...

 

I do think though that if you're going to write a data object with full read and write access and know it's going to remain that way, just write an old-fashioned plain-old-data struct. It's not "prettier" to have to write my_object.GetXXX() instead of my_object.XXX and for setters it's actually annoying if you like to do chain assignments or swaps. Qt is guilty of that a lot: you need to pass through methods to set the values of a vector or a matrix. It's just annoying. Don't do that.

 

This is kind of why I would like to see properties in C++: you can expose naked properties if you don't need to control reading and writing, and if you change your mind later and need a getter or setter, you can write them without breaking any code. But again, I disgress.

Edited by Bearhugger

Share this post


Link to post
Share on other sites

Take the new C# compiler Roslyn. The old compiler was written in C++ while Roslyn is C# with everything immutable. Conventional thinking would be that C++ would be faster than a .NET language but even with the object churn the new compiler is far far faster.

 

 

The MSVC compilers are known for how slow they execute and compile code - they were about 10x slower than Borland's compilers and even gcc is faster today. I don't know why but they have "always" been that way so it's something in their legacy design that was not dumped until Roslyn.

Given C#'s bucketing memory allocation and garbage collection they probably just pool all memory used until compilation is complete then dump everything at shutdown and that is where I suspect most of the speed-up comes from so immutability has no impact on the compilation time.

Share this post


Link to post
Share on other sites

 

Take the new C# compiler Roslyn. The old compiler was written in C++ while Roslyn is C# with everything immutable. Conventional thinking would be that C++ would be faster than a .NET language but even with the object churn the new compiler is far far faster.

 

 

The MSVC compilers are known for how slow they execute and compile code - they were about 10x slower than Borland's compilers and even gcc is faster today. I don't know why but they have "always" been that way so it's something in their legacy design that was not dumped until Roslyn.

Given C#'s bucketing memory allocation and garbage collection they probably just pool all memory used until compilation is complete then dump everything at shutdown and that is where I suspect most of the speed-up comes from so immutability has no impact on the compilation time.

 

 

There was no intention to imply that immutability had the impact, should have been more clear :) I was trying to imply that code structure and flow is the key to faster code such that even if you have the extra object churn that immutability brings it can still be faster than a badly structured program.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this