12

This question already has an answer here:

I am perplexed by comments under this answer: https://softwareengineering.stackexchange.com/a/358851/212639

A user is arguing there against the use of getters/setters and properties. He maintains that most times using them is a sign of a bad design. His comments are gaining upvotes.

Well I'm a novice programmer, but to be honest, I find the concept of properties very appealing. Why shouldn't an array allow resizing by writing to its size or length property? Why shouldn't we get all elements in a binary tree by reading its size property? Why shouldn't a car model expose its maximum speed as a property? If we're making an RPG why shouldn't a monster have its Attack exposed as a getter? And why shouldn't it expose its current HP as a getter? Or if we're making a home design software why shouldn't we allow recoloring a wall by writing into its color setter?

It all sounds so natural, so obvious to me, I could never reach a conclusion that using properties or getters/setters might be a warning sign.

Is using properties and/or getters setters usually a bad idea and if so, why?

marked as duplicate by gnat, Robert Harvey, Bart van Ingen Schenau, Pieter B, 8bittree Oct 12 '17 at 20:13

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

  • 9
    As with many programming topics, there can be degrees of zealotry, or myopia thinking all software domains are the same. This is one such topic. – whatsisname Oct 10 '17 at 20:03
  • 2
    The keyword is surprise. If the fact that a property (on a certain property) is mutable is surprising to some code that might use that object, consider making that property read-only. Things that break code are correctness and safety issues. On the other hand, information-hiding is merely a hygiene and maintainability issue. – rwong Oct 10 '17 at 20:07
  • 4
  • 1
    @immibis Thorbjorn mentioned Java just before your comment; there's a second reason getters/setters are common in that language, related to your comment: compatibility if a former field needs to gain some logic. Python could just turn it into a property and all the existing code could use the updated version just fine, but Java (at least older versions; maybe it's different now) could not, so using getters/setters was a hedge against just such a change. – Izkata Oct 11 '17 at 4:19
  • 2
    Don't sweat it - some developers hear "getters/setters are not always the best way to model an interaction" and think it means "getters/setters are always bad", and then such misunderstandings spreads. It all comes down to context. – JacquesB Oct 11 '17 at 7:25
20

Simply exposing fields – whether as public fields, properties, or accessor methods – can be an indicator of insufficient object modelling. The result is that we ask an object for various data and make decisions on that data. So these decisions will be made outside of the object. If this happens repeatedly, maybe that decision should be the responsibility of the object, and should be provided by a method of that object: We should tell the object what to do, and it should figure out how to do that on its own.

Of course this is not always appropriate. If you overdo this, your objects end up with a wild collection of many small unrelated methods that are only used once. Maybe, it is better to keep your behaviour separate from your data and choose a more procedural approach. That is sometimes called an “anemic domain model”. Your objects then carry very little behaviour, but expose their state through some mechanism.

If you expose any kind of properties, you should be consistent, both in naming and in technique used. How to do this depends mostly on the language. Either expose all as public fields, or all via properties, or all via accessor methods. E.g. don't combined a rectangle.x field with a rectange.y() method, or a user.name() method with a user.getEmail() getter.

One issue with properties and especially with writeable properties or setters is that you risk weakening the encapsulation of your object. Encapsulation is important because you can reason about correctness of your object in isolation. This modularization allows you to comprehend complex systems more easily. But if we access fields of an object instead of issuing high-level commands, we become increasingly coupled to that class. If arbitrary values may be written to a field from outside an object, it becomes very difficult to maintain a consistent state for the object. It is the responsibility of the object to keep itself consistent, and that means validating incoming data. E.g. an array object must not have its length set to a negative value. This kind of validation is impossible for a public field and easy to forget for an autogenerated setter. It is more obvious with a high-level operation like array.resize().

Suggested search terms for further info:

  • 1
    Another problem with exposing values is that it makes it hard to change the implementation. If you have a getter int millisecondsSince1970() and decide that it needs to be a long or a String, you will probably have to change many many places elsewhere in the code. Or, if a String getName() became FancyNameObject getName(); Now, often there isn't really a good workaround, but if nobody "knew" that you were using an int internally, nobody would assume that in their code. – user949300 Oct 10 '17 at 23:51
3

It is not an inherently bad design, but like any powerful feature it can be abused.

The point of properties (implicit getter and setter methods) is that they provide a powerful syntactic abstraction allowing consuming code to treat them logically as fields while retaining control for the object that defines them.

While this is often thought of in terms of encapsulation, it often has more to do with A controlling state, and B with syntactic convenience for the caller.

To get some context, we first have to consider the programming language itself. While many languages have "Properties", both terminology and the capabilities vary dramatically.

The C# programming language has a very sophisticated notion of properties on both the semantic and a syntactic level. It allows either the getter or setter to be optional, and it critically allows them to have different visibility levels. This makes a big difference if you wish to expose a fine grained API.

Consider an assembly that defines a set of data structures that are by nature cyclic graphs. Here is a simple example:

public sealed class Document
{
    public Document(IEnumerable<Word> words)
    {
        this.words = words.ToList();
        foreach (var word in this.words)
        {
            word.Document = this;
        }
    }

    private readonly IReadOnlyList<Word> words;
}

public sealed class Word
{
    public Document Document
    {
        get => this.document;
        internal set
        {
            this.document = value;
        }
    }

    private Document document;
}

Ideally, Word.Document would not be mutable at all, but I cannot express that in the language. I could have created an IDictionary<Word, Document> that mapped Words to Documents but ultimately there will be mutability somewhere

The intent here is to create a cyclic graph such that any function that is given a word can query the Word as to its containing Document by way of the Word.Document property (getter). We also want to prevent mutation by consumers after the Document has been created. The only purpose of the setter is to establish the link. It is only meant to be written once. This is hard to do because we have to create the Word instances first. By using C#'s ability to ascribe different levels of accessibility to the getter and setter of the same property, we are able to encapsulate the mutability of the Word.Document property within the containing assembly.

Code consuming these classes from another assembly will see the property as read-only (as only having a get not a set).

This example leads me to my favorite thing about properties. They can have a getter only. Whether you simply want to return a computed value or whether you want to expose only the ability to read but not to write, the use of properties is actually intuitive and simple for both the implementation and the consuming code.

Consider the trivial example of

class Rectangle
{
   public Rectangle(double width, double height) => (Width, Height) = (width, height);

   public double Area { get => Width * Height; }

   public double Width { get; private set; }

   public double Height { get; private set; }
}

Now, as I said previously, some languages have different concepts of what a property is. JavaScript is one such example. The possibilities are complex. There are a multitude of ways in which a property can be defined by an object and their are very different ways in which visibility can be controlled.

However, in the trivial case, as in C#, JavaScript allows properties to be defined such that they expose only a getter. This is wonderful for controlling mutation.

Consider:

function createRectangle(width, height) {
   return {
     get width() {
       return width;
     },
     get height() {
       return height;
     }
   };
}

So when should properties be avoided? In general, avoid setters that have logic. Even simple validation is often better accomplished elsewhere.

Going back to C#, it is often tempting to write code such as

public sealed class Person
{
    public Address Address
    {
        get => this.address;
        set
        {
            if (value is null)
            {
                throw new ArgumentException($"{nameof(Address)} must have a value");
            }
            this.address = value;
        }
    }

    private Address address;
}

public sealed class Address { }

The exception thrown is likely a surprise to consumers. After all, Person.Address is declared mutable and null is a perfectly valid value for a value of type Address. To make matters worse, an ArgumentException is being raised. The consumer may not be aware that they are calling a function and so the type of the exception adds even more to the surprising behavior. Arguably, a different exception type, such as InvalidOperationException would be more appropriate. However, this highlights where setters can get ugly. The consumer is thinking of things in different terms. There are times when this sort of design is useful.

Instead, however, it would be better to make Address a required constructor argument, or create a dedicated method for setting it if we must expose write access, and expose a property with only a getter.

I will add more to this answer.

2

Adding getters and setters reflexively (that is, without thought, not to be confused with doing it reflectively as per the question you link) is a sign of problems. First of all, if you have passthrough getters and setters for fields, why not just makes the fields public? For example:

class Foo
{
    private int _i;
    public int getData() { return _i; }
    public void setData(int i) { _i = i; };
}

What you have done above is just make it hard to use the data, and difficult to read using code. Rather than just doing foo.i += 1 you're forcing the user to do foo.setData(foo.getData() + 1), which is just obtuse. And for what benefit? Supposedly you're doing getters and setters for encapsultation and/or data control, but if you have a passthrough public getter AND setter you don't need either of those anyway. Some people argue that you might want to add data control later, but how often is that done in practice? If you have public getters and setters you're already working at a level of abstraction were you simply are not doing that. And on the off-chance that you will, you can just fix it at that point.

Public getters and setters are a sign of cargo-cult programming, where you are just following rote rules because you believe they make your code better, but haven't acctually thought about whether or not they do. If you're code base is filled with getters and setters consider why it is that you are simply not working with PODs, perhaps that is just a better way to work with data for your domain?

The criticism to the original question is not that getters or setters are a bad idea, it is that just adding public getters and setters for all fields as a matter of principle doesn't actually do anything good.

  • 1
    I agree in principle: mindless cargoculting is not good design. But if code is part of a library, I can't go back and fix it later. I need to get it right the first time or make backwards-incompatible changes. But that's completely irrelevant for applications or internal libraries where I do have that option, i.e. most code. Read-only accessor methods are also a great opportunity to only offer certain views on the data, e.g. only expose an interface not a concrete class, only expose an immutable view, etc.. But those are just indications for accessors, not a reason to use them always. – amon Oct 11 '17 at 12:35
  • But you are assuming here that getters and setters are right the first time. In practice you really never need to change. I can't think of an example where I had to. But quite honestly a library shouldn't expose such types to begin with. There are many other ways of handling encapsulation of data than by using OOP and getters/setters. – Sebastian Zander Oct 11 '17 at 16:39
  • I can think of a half dozen reasons to use properties over POD but can't think of a single reason other than some mythical need for efficiency on why using POD would be a better choice. Perhaps it's a language thing. – Dunk Oct 11 '17 at 20:13
  • @SebastianZander "why not just makes the fields public" to isolate your public interface from your private implementation. In sensible languages with properties (ivars + auto generated getters/setters), you can make properties just as easily as ivars. If the property needs to change in the future, you can provide a new implementation of the property to replace the original "pass-through" behaviour, breaking your public interface – Alexander Oct 12 '17 at 4:42
  • 1
    Almost all of this answer is specific to Java. Other language support properties directly and one does not have the absurdity of: foo.setData(foo.getData()+1) – kevin cline Oct 13 '17 at 20:09
0

To understand the issue with getters and setters, I think it's important to step back and talk about abstraction and the concept of an interface. I don't mean the programming keyword interface such as in Java, I mean interface in the more general sense. For example, if you have a microwave oven, it's got some sort of user interface that allows you to control it. Interface design is one of the most fundamental aspects of system design. It directly relates to how your system will evolve.

Let's start with a negative example. Let's say I want to store information about my inventory. So I create a class: Inventory. First of all, my requirements say I need to know how many different items there are or a certain type in the inventory so this information can be used in other parts of the program. OK let's code:

public class Inventory {
    public final String sku;
    public int numberOfItems;
}

Great, now I can create Inventory objects that can be shared in the program. But soon I run into a problem. When items are sold or added to the inventory, I need to update these objects. There could be lots of different people buying or selling these things at the same time. When two parts of the program try to update the number at the same time, we start getting errors. For example, say we have 50 items and 100 items were brought into the warehouse but at the same time, we sold 25. So two parts read the value and then modify it and write it back. If the selling code sets the value last, we now have 25 items in the object, when in reality, we have 125.

So we add a wrapper and synchronize:

public class Inventory {
    public final String sku;
    private int numberOfItems;

    public int getNumberOfItems() {
        synchronized (this) {
            return numberOfItems;
        }
    }

    synchronized public void setNumberOfItems(int number) {
        synchronized (this) {
            this.numberOfItems = number;
        }
    }
}

Great. Now two threads can't read and write at the same time. But we haven't really solved the problem. The two parts can still read from the getter (one at a time) and then write back the wrong new values (one at a time.) We haven't really gotten anywhere.

Here's a better approach:

public class Inventory {
    private final String sku;
    private int numberOfItems;

    public int getSku() {
      return sku;
    }

    public int getNumberOfItems() {
        synchronized (this) {
            return numberOfItems;
        }
    }

    public void addItems(int number) {
        synchronized (this) {
            this.numberOfItems += number;
        }
    }
}

Now we have something that actually addresses the issue. It ensures that the changes to the inventory are made one at a time.

This is a very simple example and it's not meant to show how to write good code in 2017. The point here is that in order to have a robust design, you need to focus on how your objects/classes will define their controls to other parts of the application. The definition of the internal state has very little to do with this. In the above example, let's say the company grows and now is distributed. We inventory will be stored on some remote server somewhere. There won't even be a field. In order to keep the getter/setter interface, you would need to create other helper classes which manage your beans and transfer the data to and fro. This is how procedural (e.g. COBOL) programs work. Data is shuttled around and there various programs that manipulate that data. These kinds of designs are very brittle and costly to modify because you have many disparate pieces of code that need to be modified in concert. Each part can do things that break other parts of the system.

Note that in the improved design there are still getters. Getters are definitely less problematic than setters. While I personally dislike the get prefix on things, there are times when it's best to conform to expectations. But, you can still get into trouble with getters. If you are using objects like bags of data for delivery to the parts of the code with logic, it's essentially the same issue.

I hope that helps. Comments welcome.

-1

Or if we're making a home design software why shouldn't we allow recoloring a wall by writing into its color setter?

So let's say we have a program that allows for you to see what various paint and trim combinations look like. So you create a wall class and create a setter for color. Then when the user wants to set the color of the walls, you have to loop through all the walls and set the color on each one.

Alternately, you could have the walls pull their color from a common source. Then you update that one property, and when the walls need to be redrawn, they each pull from that property.

The difference seems minor, but the loop is noisy boilerplate. In addition, why keep X copies of a value when you could have one? This kind of approach doesn't scale. These are simple cases but these kinds of issues make it difficult to impossible to use more advanced design techniques.

Why shouldn't a car model expose its maximum speed as a property?

This might be useful but let's take a more interesting example: coefficient of friction for the tires. A simple model would have that value in a property. But it really depends on the surface. Say we need to model what happens when the car hits a large puddle but only on one side of the car. We can repeatedly check where the wheel is and when it encounters a different surface, update it's coefficient of friction. But we also need to consider the normal force. So we can add some code to that loop that considers that. Also, there's air speed. And the state of the suspension.

Or, you could have the wheel calculate it's current coefficient of friction based on the information that it's able to retrieve from relevant sources. The logic is the same but it's now aligned with the wheel. We can then easily add different versions of the wheel to account for different materials, for example. If you keep all of that logic in some external loop, you'd need a bunch of case or if statements to handle all the various scenarios.

  • A user may want to have each wall in a different color. – gaazkam Oct 10 '17 at 21:02
  • @gaazkam You can always move the goalposts. I think it's obvious that the model can be adapted to meet the requirements. – JimmyJames Oct 10 '17 at 21:05
  • 1
    wonder what's the point of one user posting two answers to the same question, was there something wrong with prior one – gnat Oct 10 '17 at 21:08
  • 2
    @gnat Because they are different angles of approach. The other one is already too long. Why does the system support this feature if it's an issue? – JimmyJames Oct 10 '17 at 21:18
  • 1
    Probably I'd try to play with the Color class and maybe make Pattern a derived class of Color – or, more likely, I'd make a plain color a special case of Pattern with only one color. Anyway, AFAIK, this is how this is being done: 3D models have textures that can be set, but they don't draw themselves, drawing is the job of the engine. – gaazkam Oct 10 '17 at 21:34

Not the answer you're looking for? Browse other questions tagged or ask your own question.