When are Getters and Setters Justified

  • Getters and setters are often criticized as being not proper OO. On the other hand most OO code I've seen has extensive getters and setters.

    When are getters and setters justified? Do you try to avoid using them? Are they overused in general?

    If your favorite language has properties (mine does) then such things are also considered getters and setters for this question. They are same thing from an OO methodology perspective. They just have nicer syntax.

    Sources for Getter/Setter Criticism (some taken from comments to give them better visibility):

    To state the criticism simply: Getters and Setters allow you to manipulate the internal state of objects from outside of the object. This violates encapsulation. Only the object itself should care about its internal state.

    And an example Procedural version of code.

    struct Fridge
    {
        int cheese;
    }
    
    void go_shopping(Fridge fridge)
    {
         fridge.cheese += 5;
    }
    

    Mutator version of code:

    class Fridge
    {
         int cheese;
    
         void set_cheese(int _cheese) { cheese = _cheese; }
         int get_cheese() { return cheese; }
     }
    
    void go_shopping(Fridge fridge)
    {
         fridge.set_cheese(fridge.get_cheese() + 5);        
    }
    

    The getters and setters made the code much more complicated without affording proper encapsulation. Because the internal state is accessible to other objects we don't gain a whole lot by adding these getters and setters.

    The question has been previously discussed on Stack Overflow:

    `Getters and setters are often criticized as being not proper OO` - Citation, please.

    @Job, why because it has code? It's a good question and reeks of holy war if taken seriously.

    The code doesn't even make sense, why is set_cheese() using +=

    @mathepic, good catch. That was a typo. Now I see why I need unit tests.

    @Winston Ewert better is for the language to easily support such constructs, like ruby or C# does. I especially like ruby's metaprogramming creation of getters/setters that make unit tests pretty useless for just testing getting/setting (certainly they are useful for other things)

    @mathepic, yes its better when the language has cleaner support. But the question is if you should be accessing the data inside the class at all.

    @Winston Ewert of course you should, if you couldn't access any data then the class is useless. However obviously you shouldn't be able to access all its data, just what is crucial to the client. If the client has to work with the data it gets back, then its not crucial and should be abstracted.

    @mathepic, right. It's just that a lot of people use getters/setters to manipulate data inside objects when they should be abstracting.

    @Winston Ewert I think a lot of the problem occurs due to libraries - they can't possible provide every function you might need so they often have to have setters and getters. Languages that don't support inheriting protectedly make this far worse.

    @mathepic, I hadn't thought of that but yeah.

    @Winston Ewert: "... the question is if you should be accessing the data inside the class at all.": well, noone forces you to implement getters and setters for all member variables, you only implement those you need. That's why you use getters and setters instead of public member variables.

    @Giorgio, of course nobody is forcing you. The problem is that many people automatically implement getters and setters for every member variable and then access them just like if they had been public.

    @Winston Ewert: I do not see how the absence of getters / setters would solve this problem: there must be a way to access every piece of data otherwise it is useless; it is the task of a good design to decide which part of the data is accessible to which part of the code. If one is a bad designer he or she will be such with or without getters and setters. Just my 2 cents.

    @Giorgio, I'm claiming that you should never have getters and setters, that's why the question is titled "when are Getters and Setters justified." I do claim that getters and setters are overused, because many programmers have failed to grasp the concept of data hiding and think that getters/setters provide it.

    Try not using them at all. See what happens. IMO, critical thought happens. DTO? Nah. Data object wrapper that knows how to map itself, how to update itself in the DB, etc... with a basic generic data structure at the core. That's a lot more appealing to me than contemporary accepted thought on the matter.

    I realize this is an old post, but I have to comment. I'm seeing a lot of getters and setters in some Java code I've recently been asked to review. I don't have much experience with Java, so I wanted to verify whether it was a better programming practice than I thought. I believe the accepted answer has a flaw. We don't have to add a `setSalary()` method. Instead, we should have something like: public void acceptNewJob(Job newJob) { salary = newJob.getSalary(); } This way the salary is set from an object that makes sense, instead of any old piece of code. That's my 2 cents.

    @Job — Programmers.SE is the perfect site for this question. It doesn't belong on SO because it's not a specific problem with a specific solution; it's more of a general coding style/architecture problem. Even if it does tend to bring up debate.

    "you only implement those you need. That's why you use getters and setters instead of public member variables." -- There's no design difference between public setters and public member variables ... you can make member variables public only as needed. Both are poor design for the same reason globals are bad designe -- because control of the value of the property is not localized. Use immutable objects instead -- their properties are only set at time of construction.

  • Dima

    Dima Correct answer

    10 years ago

    Having getters and setters does not in itself break encapsulation. What does break encapsulation is automatically adding a getter and a setter for every data member (every field, in java lingo), without giving it any thought. While this is better than making all data members public, it is only a small step away.

    The point of encapsulation is not that you should not be able to know or to change the object's state from outside the object, but that you should have a reasonable policy for doing it.

    • Some data members may be entirely internal to the object, and should have neither getters nor setters.

    • Some data members should be read-only, so they may need getters but not setters.

    • Some data members may need to be kept consistent with each other. In such a case you would not provide a setter for each one, but a single method for setting them at the same time, so that you can check the values for consistency.

    • Some data members may only need to be changed in a certain way, such as incremented or decremented by a fixed amount. In this case, you would provide an increment() and/or decrement() method, rather than a setter.

    • Yet others may actually need to be read-write, and would have both a getter and a setter.

    Consider an example of a class Person. Let's say a person has a name, a social security number, and an age. Let's say that we do not allow people to ever change their names or social security numbers. However, the person's age should be incremented by 1 every year. In this case, you would provide a constructor that would initialize the name and the SSN to the given values, and which would initialize the age to 0. You would also provide a method incrementAge(), which would increase the age by 1. You would also provide getters for all three. No setters are required in this case.

    In this design you allow the state of the object to be inspected from outside the class, and you allow it to be changed from outside the class. However, you do not allow the state to be changed arbitrarily. There is a policy, which effectively states that the name and the SSN cannot be changed at all, and that the age can be incremented by 1 year at a time.

    Now let's say a person also has a salary. And people can change jobs at will, which means their salary will also change. To model this situation we have no other way but to provide a setSalary() method! Allowing the salary to be changed at will is a perfectly reasonable policy in this case.

    By the way, in your example, I would give the class Fridge the putCheese() and takeCheese() methods, instead of get_cheese() and set_cheese(). Then you would still have encapsulation.


    public class Fridge {
      private List objects;
      private Date warranty;
    
      /** How the warranty is stored internally is a detail. */
      public Fridge( Date warranty ) {
        // The Fridge can set its internal warranty, but it is not re-exposed.
        setWarranty( warranty );
      }
    
      /** Doesn't expose how the fridge knows it is empty. */
      public boolean isEmpty() {
        return getObjects().isEmpty();
      }
    
      /** When the fridge has no more room... */
      public boolean isFull() {
      }
    
      /** Answers whether the given object will fit. */
      public boolean canStore( Object o ) {
        boolean result = false;
    
        // Clients may not ask how much room remains in the fridge.
        if( o instanceof PhysicalObject ) {
          PhysicalObject po = (PhysicalObject)o;
    
          // How the fridge determines its remaining usable volume is a detail.
          // How a physical object determines whether it fits within a specified
          // volume is also a detail.
          result = po.isEnclosedBy( getUsableVolume() );
        }
    
         return result;
      }
    
      /** Doesn't expose how the fridge knows its warranty has expired. */
      public boolean isPastWarranty() {
        return getWarranty().before( new Date() );
      }
    
      /** Doesn't expose how objects are stored in the fridge. */
      public synchronized void store( Object o ) {
        validateExpiration( o );
    
        // Can the object fit?
        if( canStore( o ) ) {
          getObjects().add( o );
        }
        else {
          throw FridgeFullException( o );
        }
      }
    
      /** Doesn't expose how objects are removed from the fridge. */
      public synchronized void remove( Object o ) {
        if( !getObjects().contains( o ) ) {
          throw new ObjectNotFoundException( o );
        }
    
        getObjects().remove( o );
    
        validateExpiration( o );
      }
    
      /** Lazily initialized list, an implementation detail. */
      private synchronized List getObjects() {
        if( this.list == null ) { this.list = new List(); }
        return this.list;
      }
    
      /** How object expiration is determined is also a detail. */
      private void validateExpiration( Object o ) {
        // Objects can answer whether they have gone past a given
        // expiration date. How each object "knows" it has expired
        // is a detail. The Fridge might use a scanner and
        // items might have embedded RFID chips. It's a detail hidden
        // by proper encapsulation.
        if( o implements Expires && ((Expires)o).expiresBefore( today ) ) {
          throw new ExpiredObjectException( o );
        }
      }
    
      /** This creates a copy of the warranty for immutability purposes. */
      private void setWarranty( Date warranty ) {
        assert warranty != null;
        this.warranty = new Date( warranty.getTime() )
      }
    }
    

    Please don't take my Fridge example too seriously. :) A real fridge ought to be a container object, I'd expect it to know how to hold objects without worrying about what exactly they were. I.E. it would be like an ArrayList. As for what makes it a Fridge, let's say it serializes objects to disk when stored so that they will survive system failure. Ok. Enough taking my fridge example over seriously.

    On the contrary, I think a smart fridge should know what it contains and how much. This way, if you call `takeCheese()` and there is no cheese left, the fridge would throw an exception. :) The point is that you want to create abstractions, and the class interface is what lets you do that. So you want your interface to represent the things you want your object to do, not its internal structure. Sometimes the two are essentially the same, but more often they are not.

    oh, the fridge knows what it contains. But it doesn't understand the nature of what it contains. The interface would be something like take(Cheese, 5) to remove 5 units of cheese from the fridge. The fridge has no knowledge of actual cheese, milk, or anything else. It just stores what is put into it.

    But shouldn't the fridge know the expiration dates, so that it can tell you that the cheese has gone bad and should be thrown out? :) Ok, we have to stop this! :)

    Quite an interesting discussion of Fridge Logic you've got going on here...

    Haha, I'd like to see the ad for that: **"It's a brand new kind of fridge: It throws things at you when trying to grab something that isn't there! This way you will only try that once! You worry about stuffing the fridge, and let the fridge worry about illegal behaviour!"**

    +1 This explanantion is better than any I found in books.

    Ah, but what happens when the person's data is just wrong. The customer calls up and says "you've enterd my age incorrectly, it says I'm 76 when I'm actually 22". Suddenly you have to tell them to get used to it, or whip out an SQL editor and directly modify the DB. However, I understand what you're saying - getters and setters are bad; give an object methods that perform defined actions on it.

    @gbjbaanb, this really depends on how the whole system is implemented. Maybe there is a class called PersonParams, which is plain old data with public fields or getters/setters for everything, and it is this object that is used to initialize Person. Or maybe you just delete the person's record and create a new one. This is not the point. And no, I am not saying that getters/setters are bad. I am saying that you need to look at your class and decide which data members should be read-only, which should be read-write, and which should be completely hidden.

    "By the way, in your example, I would give the class Fridge the putCheese() and takeCheese() methods, instead of get_cheese() and set_cheese(). Then you would still have encapsulation." This explains a lot! Credits should be shared between you both as, Winston Ewert : For putting it as get_cheese() and set_cheese() Dima : For putCheese() and takeCheese() Before I came here I went through http://programmers.stackexchange.com/questions/187886/how-important-is-encapsulation/187894#187894 and after reading this part of your answer, I understood most of the things there were in that answer too.

    The example has it all wrong. There should not be an Age field nor a setAge() method. Age is a function of the Person birthDate as compared to some point in time. While seemingly trivial, this is exactly what is wrong with the modern practice of full mutability of objects and other bad designs, as seen by get/set methods for private fields versus carefully considering what is really mutable, what is a field, what the object should know about its behaviors and what state changes are actually valid (which setX methods completely destroy).

    +1. But for the salary, shouldn't the job be its own object, complete with its own salary field, and shouldn't the person's salary be determined by the salary field inside of their job object?

    +1 for Darrell and getting the underlying design right to minimise the need for mutability. Of course one may be hamstrung by a poor database design and "just make it work, now" attitudes from above...

    what about having no getter and setter for some variables, and having them public, does that "break encapsulation" and does it matter?

    Yes, public data members do break encapsulation, and are generally a bad idea, because they create tight coupling between classes. Getters and setters provide a better degree of separation between the implementation and the interface of your class, and give you more wiggle room to change the implementation without changing the interface.

    "Having getters and setters does not in itself break encapsulation. What does break encapsulation is having a getter and a setter for every data member (every field, in java lingo)." -- This is obviously nonsense; the percentage of an object's properties that are externally changeable has no bearing on whether encapsulation is broken. The plain fact is that public setters break encapsulation of the *value* of the property ... any piece of code anywhere can change it at any time. Darrell Teague gets it right ... public setters are bad design.

    All I get from that and your answer is that you don't understand the concept of encapsulation. it also contradicts your *assertion* that "Having getters and setters does not in itself break encapsulation". That claim is quite different from "they only break encapsulation a little if you only have a few of them". Again, Darrell Teague got it right. You have no response to that because of your lack of expertise in this area.

    Immutability is not the arbiter of good design. The reason Age shouldn't be a field has little to do with immutability and everything to do with avoiding a fragile, costly, and potentially bug-ridden nightmare going forward. While there is a correlation between immutability and good design in this case, correlation is not causation. Private fields may commonly be of no concern to the outside world, but that's by no means the only reason to make a field private. Contrary to what trendy Functional Programming purists say, merely changing state from outside doesn't violate encapsulation.

    @dave-jarvis Can you give some creditable citation for this, "What does break encapsulation is automatically adding a getter and a setter for every data member" as I am facing very hard time explaining this to the people and already been rejected from few interviews as more than half dev population defines encapsulation as creating getter setter of fields/member. Thanks

    @foxt7ot I learned this from "Effective C++" by Scott Meyers. Also, a word of advice: an interview is a bad time to argue about programming philosophy.

    @Dima Thanks for citation and your valuable advice

    The salary example is misleading. In that case, the method should be `changeJob(newSalary)` or `getRaise(newSalary)`, etc. (with possible additional parameters). See Cormac's answer.

License under CC-BY-SA with attribution


Content dated before 6/26/2020 9:53 AM