How to avoid getters and setters

Question

I have read in many places that "getters and setters are evil". And I understood why so. But I don't know how to avoid them completely. Say Item is a class that has information about item name, qty, price etc... and ItemList is a class, which has a list of Items. To find the grand total:

int grandTotal()
{
int total = 0;

for (Item item: itemList)
       total += item.getPrice();

return total;
}

In the above case, how does one avoid getPrice()? The Item class provides getName, setName, etc....

How do I avoid them?

Solution 1

When should you use getters and setters?

Getters and setters are great for configuring or determining the configuration of a class, or retrieving data from a model

Getting the price of an item is an entirely reasonable use of a getter. That is data that needs to be available and may involve special considerations to protect the data by adding validation or sanitization to the setter.

You can also provide getters without setters. They do not have to come in pairs.

When shouldn't you use getters and setters?

Sometimes objects rely on internal properties that will never be exposed. For example, Iterators and internal collections. Exposing the internal collection could have dramatically negative and unexpected consequences.

Also, for example, let's say you are communicating via some HttpURLConnection. Exposing the setter for your HttpURLConnection means that you could end up with a very odd state should the connection be changed while waiting to receive data. This connection is something that should be created on instantiation or entirely managed internally.

Summary

If you have data that is for all intents and purposes public, but needs to be managed: use getters and setters.

If you have data that needs to be retrieved but under no circumstances should ever be changed: use a getter but not a setter.

If you have data that needs to be set for internal purposes and should never be publicly exposed (and cannot be set at instantiation): use a setter but not a getter (setter presumably prevents a second call affecting the internal property)

If you have something that is entirely internal and no other class needs to access it or change it directly, then use neither.

Don't forget that setters and getters can be private and even for internally managed properties, having a setter that manages the property may be desirable. For example, taking a connection string and passing it to the setter for HttpURLConnection.

Also note:

Allen Holub's article Why getter and setter methods are evil seems to be the source of OP's reasoning but, in my opinion, the article does a poor job of explaining its point.

Edit: Added summary
Edit 2: spelling corrections

Solution 2

It's a shame to see a small, vocal minority take a back lash against the whole "Getters and Setters" are evil debate. Firstly the article title is purposely provocative to draw you in, as should any blog post. I've in turn blogged about this before and several years later updated my opinions and ideas about this question. I'll summarise the best I can here.

• Getters and setters (accessors) are not evil
• They are "evil" (unnecessary) most of the time however
• Encapsulation is not just adding accessors around private fields to control change, after all there is no benefit to added get/set methods that just modify a private field
• You should write as much code as possible with the principle of "Tell, Don't Ask"
• You need to use accessors for framework code, DTOs, serialisation and so forth. Don't try to fight this.
• You want your core domain logic (business objects) to be as property free as possible however. You should tell objects to do stuff, not check their internal state at will.

If you have a load of accessors you essentially violate encapsulation. For example:

class Employee
{
   public decimal Salary { get; set; }

   // Methods with behaviour...
}

This is a crap domain object, because I can do this:

me.Salary = 100000000.00;

This may be a simple example, but as anyone who works in a professional environment can attest to, if there is some code that is public people will make use of it. It would not be wrong for a developer to see this and start adding loads of checks around the codebase using the Salary to decide what do with the Employee.

A better object would be:

class Employee
{
   private decimal salary;

   public void GivePayRise()
   {
        // Should this employee get a pay rise.
        // Apply business logic - get value etc...
        // Give raise
   }

   // More methods with behaviour
}

Now we cannot rely on Salary being public knowledge. Anyone wanting to give a pay rise to employees must do this via this method. This is great because the business logic for this is contained in one place. We can change this one place and effect everywhere the Employee is used.

Solution 3

The following sample is a brilliant example of boilerplate setters and getters.

class Item{
  private double price;

  public void setPrice(final double price){
    this.price = price;
  }
  public double getPrice(){
    return this.price;
  }
}

Some coders think that this is called encapsulation, but in fact this code is exact equivalent of

class Item{
  public double price;
}

In both classes price is not protected or encapsulated, but the second class reads easier.

 class Item{
    private double price;

    public void setPrice(final double price){
      if(isValidPrice(price))
        this.price = price;
      else throw new IllegalArgumentException(price+" is not valid!");
    }

    public double getPrice(){
      return this.price;
    }
  }

This is a real encapsulation, the invariant of the class is guarded by the setPrice. My advice - don't write dummy getters and setters, use getters and setters only if they guard the invariant of your class

Solution 4

I have read in many places that "getters and setters are evil".

Really? That sounds crazy to me. Many? Show us one. We'll tear it to shreds.

And I understood why so.

I don't. It seems crazy to me. Either your misunderstood but think you did understand, or the original source is just crazy.

But I don't know how to avoid them completely.

You shouldn't.

how to avoid getPrice?

See, why would you want to avoid that? How else are you suppose to get data out of your objects?

how to avoid them???

Don't. Stop reading crazy talk.

Solution 5

When someone tells you that getters and setters are evil, think about why they are saying that.

Getters

Are they evil? There is no such thing as evil in code. Code is code and is neither good nor bad. It's just a matter of how hard it is to read and debug.

In your case, I think it is perfectly fine to use a getter to calculate the final price.

The "evil"

Usecase: you think you want the price of an item when buying something.

People sometimes use getters like this:

if(item.getPrice() <= my_balance) {
   myBank.buyItem(item);
}

There is nothing wrong with this code, but it isn't as straight-forward as it could be. Look at this (more pragmatic approach):

myBank.buyItem(item); //throws NotEnoughBalanceException

It's not the buyers or the cashiers job to check the price of an item when buying something. It's the actually the bank's job. Imagine that customer A has a SimpleBank.java

public class SimpleBank implements Transaction {

  public void buyItem(Item item){
     if(getCustomer().getBalance() >= item.getPrice()){
        transactionId = doTransaction(item.getPrice());
        sendTransactionOK(transactionId);
     }
  }
}

The first approach seems fine here. But what if customer B has a NewAndImprovedBank.java?

public class NewAndImprovedBank implements Transaction {

  public void buyItem(Item item){
     int difference = getCustomer().getBalance() - item.getPrice();
     if (difference >= 0) {
        transactionId = doTransaction(item.getPrice());
        sendTransactionOK(transactionId);
     } else if (difference <= getCustomer().getCreditLimit()){
        transactionId = doTransactionWithCredit(item.getPrice());
        sendTransactionOK(transactionId);
     }
  }
}

You might think that you are being defensive when using the first approach, but actually you are limiting the capabilities of your system.

Conclusion

Don't ask for permission aka item.getPrice() , ask for forgiveness aka NotEnoughBalanceException instead.

View more solutions

Comments

[Jerry Coffin]

Is your Item really anything more than raw data?

[Wes]

Who says they're "evil"? Maybe the lazy people who don't want to write an extra 40 methods per class. I'd say making private variables public, is more "evil".

[Andres F.]

@Wes Getters/setters being evil and breaking encapsulation is a well-known case argued by Allen Holub in JavaWorld: Why getter and setter methods are evil. Before dismissing the concern, read about it -- it's not at all about the extra keystrokes.

[Pradeep Kumar]

@AndresF. I've come across his writing before, (his article on extends specifically) and while he makes some good points, he couches them in inflammatory language. Both the extends vs. contains and getters/setters subjects are important with lots of shades of gray so it is unfortunate to see his articles inspire these kinds of religious debates. Novice programmers are especially vulnerable to seeing absolutes and accepting them as gospel (so to speak).

[SWeko]

This seems to be the canonical article, but yes, it's crazy talk.

[JustinW]

Totally agree. For more info check out LBushkin's answer in this link stackoverflow.com/questions/1568091/why-use-getters-and-sett‌​ers

[Andres F.]

-1 For not making the effort to understand the issue. Indeed, Allen Holub's article is the canonical source, but it's not crazy talk. It's a valid concern. That we're taught to blindly write tons of getters and setters doesn't make it the best way to write Java code

[Andres F.]

-1 For not reading the article by Allen Holub. The alternative to using getters/setters is not to expose internal attributes, but to reconsider your design.

[Matt Gibson]

There's also scope for permissions control as per the proxy pattern.

[jason]

@Andres F.: Pardon? 1. It's on the OP to provide the context; I told the OP in my answer to provide the context so we can dissect it. 2. The OP is not tagged as Java. 3. The OP didn't link to the Holub article, downvoting someone for not reading it is asinine; I'd downvote your comment if I could. 4. No one sensible is teaching blindly writing of getters and setters.

[Kiril Kirilov]

True. My example seems a little bit dumb in the context of some enterprise network which creates a proxies of injected classes (@EJBs or @Components or whatever).

[Pradeep Kumar]

The one problem is that you assume at the time of writing that you know all of the validation and error checking you want to do. The point of writing the simple case of the setter is that you can add argument transformations/validation/etc. later on without changing the call to someitem.setPrice(price). Adding Exceptions obviously creates knock-on effects, but that would be true in any event.

[Andres F.]

@Jason Exposing object internals is bad OO style regardless of programming language. And you must provide a good quality answer, which you didn't -- I'm not downvoting you because I disagree, but because you didn't bother to research the issue and dismissed it as "crazy talk". A well-researched answer would have noted the design problem, and explained why in some cases breaking encapsulation is unavoidable.

[Kiril Kirilov]

@Jonathan With modern IDEs, renaming or creating getters/setters is matter of seconds. And getter/setters allow more precise debugging. My point is that dummy getters/setters make the code harder to read.

[jason]

@Andres F.: And I did read the article, and I do think he is crazy. People walk away thinking he's saying "never use getters/setters" because he says explicitly in the "extends is evil" artcile that "You should never use get/set functions" and that is crazy. Additionally, he says "You shouldn't use accessor methods (getters and setters) unless absolutely necessary." What an utterly trivial but useless statement.

[Pradeep Kumar]

@AndresF. While the article makes some good points, it is full of over the top assertions and is incredibly long-winded. He says "You shouldn't use accessor methods (getters and setters) unless absolutely necessary" on the last page. And he's right. He fails to give good examples which make his point significantly less clear. His central point is that you need to consider what the class is for and do that while exposing as little detail as possible about the internal workings. But the abysmal writing leads to cases like OP that are simply scared off of getters and setters all together

[Wes]

Pardon? Who cares if I didn't read some article on some website somewhere. There's shit written all over the Internet. He asked for an answer on "how do I" and I showed him a way "how". +1 IMHO.

[jason]

@Andres F.: Finally, he's completely confused in thinking that accessors are implementation details. Sorry, he is crazy.

[GaryJL]

@AndresF. That Article was not referenced at all when this answer was posted. The question didn't even refer to it, only your comment 4 minutes after this answer was posted made the first reference. Surely an expectation to have read it is unrealistic, a down-vote is definitely ridiculous.

[Pradeep Kumar]

@Jason the one point I would make in favor of Holub's article is that classes sometimes have variables they keep track of but are only ever for internal use and never need to be exposed (e.g. Iterators and their internal collections). For those cases, a getter or setter would be undesirable. Holub just does a terrible job of explaining that.

[Admin]

The importance of getters and setters is to provide protection to private attributes so that they can not be accessed directly for this it is best that you create a class with the attribute item in which you include the corresponding get and set.

[Andres F.]

Understood :) But consider fiction's reply: if you simply wrap every private attribute with a getter/setter pair, are you really "protecting" them? Or simply adding boilerplate?

[jason]

@Jonathan Fingland: Sure, but that's a completely different claim than "avoid getters/setters because they're evil."

[Pradeep Kumar]

@Jason Oh, I absolutely agree as my comments here would indicate. Holub's article has a good point but is couched in inflammatory language and abolutes, and then he concludes with by contradicting the absolutes. Basically, the article is poorly written and has an awful title. It would have been far better to say: There are cases when getters and setters should be avoided. Here are some examples "...", carefully consider which getters and setters are needed in your classes and not simply add them for the sake of completeness.

[jason]

@Andres F.: But no one is advocating wrapping every private field with a public getter/setter. You're setting up a straw man that no one would support, and then tearing it down. So what?

[Andres F.]

@Jason I know it's not what Margge said, I simply want her to elaborate. I was merely arguing that getters/setters, by themselves, do not really provide protection. Having a public property or having it private but wrapped in accessors is not so different.

[Finglas]

Yet another person misunderstands the point that "Getters and Setters" are evil.

[jason]

@Finglas: Oh, please enlighten us and state why you think I misunderstand.

[dan04]

@fiction: You can't change other people's code that uses your class in a matter of seconds.

[Stefan Hendriks]

One question though: What if the payrise depends on other factors, like the amount of Employees in the company, or the status of the economy? Would you give the Employee knowledge of this information? Or would you (atleast, I would do it) delegate this to a different class? But if you would delegate, the only way for that class to know is to get the information from other classes?

[Pradeep Kumar]

I'm going to b honest here -- your example hurts yours case. There are numerous use cases where getting and setting salaries is desirable. Better to check constraints than simply eliminate the option. Does the user or object making the request have the authority to do so? In your example, you use empty getters and setters, which is fine for stubbing out functions you plan to expand on, but is entirely not the point of getters and setters. Use getters and setters when there needs to be data access or manipulation and there needs to be some management, validation, or sanitization going on.

[Finglas]

@JonathanFingland the first example is void of methods, but imagine there was logic associated with the class. In other words the example would have behavior. Getters/Setters with logic e.g. object cannot set this to a specific value usually hints at much more complex logic than simply Get/Set a value, hence it should be a method.

[Finglas]

@StefanHendriks the example was just spur of the moment but following a DDD style of thinking a service or services would be involved yes. The point is that something would tell each employee to receive a pay rise, then each employee/service would do the right thing. They'd know how long they have worked, what level and so forth.

[Pradeep Kumar]

setSalary(salary) { //get session credentials; //check permissions; //update salary value; //log update } just an example obviously, but I think it gets the point across. Sometimes all you want to do is set the value in a managed way. Giving a pay raise is not necessarily a bad function, but it is not the same as setting the salary.

[Finglas]

@JonathanFingland yes it is not the same. My point was to demonstrate how you would increase/decrease an Employee's salary. NOT get/set their salary. It makes no sense to arbitrarily set an Employees salary in terms of a employee management domain.

[Nageswaran]

I didn't ask how to hide, I asked how to avoid.

[CurtainDog]

I'm not sure what you're trying to do, but using doubles is almost certainly the wrong way to do it.

[Elias]

Your answer clarifies a bit better what Allen Holub's article generally was getting at. I was having a lot of trouble understanding its point, and the solution he was trying to provide in it's stead.

[TheSecretSquad]

If you read Allen Holub's book "Holub on Patterns", he says that he's not against return values. He advocates passing as little data as possible, and where you can, "push" data into methods of collaborating objects instead of "pulling" data out to operate on. He suggests that if you're returning a value it should be a type that is a key abstraction in the domain. With this in mind, the return type of getPrice() in this example should probably be a Money type to hide potential changes of the price's type from say, double to BigDecimal, or something else.

[Kiril Kirilov]

@TheSecretSquad Couldn't agree more

[Arturas M]

@fiction I would agree with you, but I've also heard somewhere that getters and setters should only set and get the value, without actually doing any serious checking or calculations, because that's not what we or some frameworks could expect them to do. I guess the last idea would suggest this setPrice() in your last example to be called somewhat different, maybe "setPriceIfValid()"? What's your opinion on that mindset?

[Giumo]

But, why would anyone set an invalid price? If it's from the user, then the validation should be done just after reading form the user.

[IcedDante]

The problem with avoiding setters is that now you have to set the data via a constructor. As soon as you have a constructor that take more than 5 arguments looking and modifying this becomes unwieldy

[LAFK says Reinstate Monica]

Comment would suffice. "This is crazy talk. don't listen to crazy people, you read crazy articles". Not really an answer.