30

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?

Finglas
  • 15,518
  • 10
  • 56
  • 89
Nageswaran
  • 7,481
  • 14
  • 55
  • 74
  • Is your `Item` really anything more than raw data? – Jerry Coffin Feb 23 '12 at 15:43
  • 8
    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". – Wes Feb 23 '12 at 15:44
  • 10
    @Wes Getters/setters being evil and breaking encapsulation is a well-known case argued by [Allen Holub](http://en.wikipedia.org/wiki/Allen_Holub) in JavaWorld: [Why getter and setter methods are evil](http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html). Before dismissing the concern, read about it -- it's not at all about the extra keystrokes. – Andres F. Feb 23 '12 at 15:54
  • 1
    @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). – Jonathan Fingland Feb 23 '12 at 18:09

15 Answers15

47

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

Community
  • 1
  • 1
Jonathan Fingland
  • 56,385
  • 11
  • 85
  • 79
  • 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. – Elias Mar 03 '14 at 15:30
  • 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 – IcedDante May 27 '15 at 20:38
20

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.

Finglas
  • 15,518
  • 10
  • 56
  • 89
  • 1
    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? – Stefan Hendriks Mar 01 '12 at 09:16
  • 1
    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. – Jonathan Fingland Mar 01 '12 at 15:20
  • @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 Mar 01 '12 at 21:28
  • @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. – Finglas Mar 01 '12 at 21:31
  • 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. – Jonathan Fingland Mar 02 '12 at 18:15
  • 2
    @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. – Finglas Mar 03 '12 at 10:56
16

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

TheSecretSquad
  • 303
  • 2
  • 14
Kiril Kirilov
  • 11,167
  • 5
  • 49
  • 74
  • 1
    There's also scope for permissions control as per the proxy pattern. – Matt Gibson Feb 23 '12 at 16:02
  • 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). – Kiril Kirilov Feb 23 '12 at 16:12
  • 8
    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. – Jonathan Fingland Feb 23 '12 at 16:12
  • 2
    @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. – Kiril Kirilov Feb 23 '12 at 16:16
  • 4
    @fiction: You can't change other people's code that uses your class in a matter of seconds. – dan04 Feb 29 '12 at 22:08
  • I'm not sure what you're trying to do, but using doubles is almost certainly the wrong way to do it. – CurtainDog Mar 08 '13 at 23:51
  • 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. – TheSecretSquad Jun 19 '14 at 03:11
  • @TheSecretSquad Couldn't agree more – Kiril Kirilov Jun 19 '14 at 10:01
  • @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? – Arturas M Aug 21 '14 at 15:21
  • 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. – Giumo Oct 04 '14 at 15:07
7

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.

jason
  • 236,483
  • 35
  • 423
  • 525
  • 3
    [This](http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html) seems to be the canonical article, but yes, it's crazy talk. – SWeko Feb 23 '12 at 15:46
  • 2
    Totally agree. For more info check out LBushkin's answer in this link http://stackoverflow.com/questions/1568091/why-use-getters-and-setters – JustinW Feb 23 '12 at 15:46
  • 6
    -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. Feb 23 '12 at 15:56
  • 3
    @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. – jason Feb 23 '12 at 16:08
  • 8
    @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. – Andres F. Feb 23 '12 at 16:13
  • 1
    @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. – jason Feb 23 '12 at 16:20
  • 2
    @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 – Jonathan Fingland Feb 23 '12 at 16:22
  • @Andres F.: Finally, he's completely confused in thinking that accessors are implementation details. Sorry, he is crazy. – jason Feb 23 '12 at 16:26
  • 2
    @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. – Jonathan Fingland Feb 23 '12 at 16:52
  • @Jonathan Fingland: Sure, but that's a completely different claim than "avoid getters/setters because they're evil." – jason Feb 23 '12 at 19:18
  • 2
    @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.* – Jonathan Fingland Feb 23 '12 at 19:26
  • 2
    Yet another person misunderstands the point that "Getters and Setters" are evil. – Finglas Feb 29 '12 at 21:34
  • @Finglas: Oh, please enlighten us and state why you think I misunderstand. – jason Feb 29 '12 at 21:36
  • 2
    Comment would suffice. "This is crazy talk. don't listen to crazy people, you read crazy articles". Not really an answer. – LAFK 4Monica_banAI_modStrike Nov 29 '15 at 10:37
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.

Community
  • 1
  • 1
Arnab Datta
  • 5,356
  • 10
  • 41
  • 67
3

getPrice() is accessing a private variable I'm assuming.

To answer your question directly, make the price variable public, and code something like (syntax may differ depending on language, use of pointers etc):

total += item.price;

However this is generally considered bad style. Class variables should generally remain private.

Please see my comment on the question.

Wes
  • 4,781
  • 7
  • 44
  • 53
  • 1
    -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. – Andres F. Feb 23 '12 at 15:59
  • 5
    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. – Wes Feb 23 '12 at 16:25
  • 5
    @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. – GaryJL Feb 23 '12 at 16:32
2

How to avoid getters and setters? Design classes that actually act upon the data they hold.

Getters lie about the data anyway. In the Item.getPrice() example, I can see I'm getting an int. But is the price in dollars or cents? Does it include tax(es)? What if I want to know the price in a different country or state, can I still use getPrice()?

Yes, this might be beyond the scope of what the system is designed to do, and yes, you might just end up returning a variable's value from your method, but advertising that implementation detail by using a getter weakens your API.

CurtainDog
  • 3,175
  • 21
  • 17
2

'Evil' as .getAttention()

This has been discussed often, and even perhaps went a bit viral, as a result of the pejorative term "Evil" used in the dialog. There are times when you need them, of course. But the problem is using them correctly. You see, Professor Holub's rant isn't about what your code is doing now, but about boxing yourself in so that change in the future is painful and error prone.

In fact, all I have read by him carries this as its theme.

How does that theme apply to the class Item?

A look at the future of Item

Here is fictions's item class:

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 all well and good- but it is still 'Evil' in the sense that it could cause you a lot of grief in the future.

The grief is apt to come from the fact that one day 'price' may have to take different currencies into account (and perhaps even more complex barter schemes). By setting price to be a double, any code that is written between now and the 'apocalypse' (we're talking evil, after all) will be wiring price to a double.

It is much better (even Good, perhaps) to pass in a Price object instead of a double. By doing so you can easily implement changes to what you mean by 'price' without breaking the existing interfaces.

The takeaway on getters and setters

If you find yourself using getters and setters on simple types, make sure you consider possible future changes to the interface. There is a very good chance you shouldn't be. Are you using setName(String name)? You should consider setName(IdentityObject id) or even setIdentity(IdentityObject id) in case other identification models show up (avatars, keys, whatever). Sure you can always go around and setAvatar and setKey on everything, but by using an object in your method signature you make it easier to extend in the future to the objects that can use the new identity properties and not break the legacy objects.

dhimes
  • 81
  • 1
  • 3
2

A different perspective that is missing here so far: getters and setters invite to violate the Tell Don't Ask principle!

Imagine you go shopping in the supermarket. In the end, the cashier wants money from you. The getter/setter approach is: you hand over your purse to the cashier, the cashier counts the money in your purse, takes the money you owe, and gives back the purse.

Is that how you do things in reality? Not at all. In the real world, you typically don't care about the internal state of "autonomous" other "objects". The cashier tells you: "your bill is 5,85 USD". Then you pay. How you do that is up to you, the only thing the cashier wants/needs is he receives that amount of money from your side.

Thus: you avoid getters and setters by thinking in terms of behavior, not in terms of state. Getters/setters manipulate state, from the "outside" (by doing avail = purse.getAvailableMoney() and purse.setAvailableMoney(avail - 5.85). Instead, you want to call person.makePayment(5.85).

GhostCat
  • 137,827
  • 25
  • 176
  • 248
1

Cloudanger answer is is one, but you must also realize that the item list will likely contain many item objects with quantity ordered on it.

Solution : create another class in between them that stores your item in the item list and the qty ordered for that item (Let's say the class is called OrderLine).

OrderLine will have Item and qty as fields.

After that, code something like calculateTotal(int qty) in Item which return price*qty. Create a method in OrderLine that call calculateTotal(qtyOrdered)

Pass the return value to the itemList.

This way, you avoid getters. The ItemList will only know the total price. Your code should live with your data. Ask the Object who has the data to calculate the totalPrice instead of asking that object for raw data to calculate your totalPrice.

1

How to avoid getters and setters in Java? Use Project Lombok

Alex Bitek
  • 6,529
  • 5
  • 47
  • 77
0

Use a helper class ShoppingCart. Item's method item.addTo(ShoppingCart cart) would add the price to the totalSum of the cart using shoppingCart.addItem(Item item, int price)

Dependency from Item to ShoppingCart isn't disadvantageous if the Items are meant to be items of ShoppingCarts.

In the case where Items live solely for the ShoppingCart and the Item class is small, I would more likely have the Item as an inner class of the ShoppingCart, so that the ShoppingCart would have access to the private variables of the items.

Other thoughts

It would also be possible, although quite unintuitive design, to have the Item class count the sum (item.calculateSum(List<Item> items)), since it can access the private parts of other items without breaking encapsulation.

To others wondering why the getters are bad. Consider the given example where the getPrice() returns integer. If you would want to change that to something better like BigDecimal at least or a custom money type with currency, then it wouldn't be possible since the return type int exposes the internal type.

Cloudanger
  • 9,194
  • 2
  • 22
  • 18
0

Getters and setters are evil because they break encapsulation and can unnecessarily expose an objects internal state and allow it to be modified in way it should not be. The following article elaborates on this problem:

http://programmer.97things.oreilly.com/wiki/index.php/Encapsulate_Behavior,_not_Just_State

murungu
  • 2,090
  • 4
  • 21
  • 45
0

Really? I don't think that. on the contrary the getters and setters help you to protect the consistense of the variables.

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.

  • 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. –  Feb 23 '12 at 17:25
  • Understood :) But consider [fiction's reply](http://stackoverflow.com/a/9416499/147346): if you simply wrap every private attribute with a getter/setter pair, are you really "protecting" them? Or simply adding boilerplate? – Andres F. Feb 23 '12 at 17:35
  • @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? – jason Feb 23 '12 at 21:52
  • @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_. – Andres F. Feb 23 '12 at 22:29
-1

You can avoid getter and setter at places by using _classname__attributename because that's the changed new name once you declare private to any attribute.

So if Item is the class with a private attribute declared as __price

then instead of item.getPrice() you can write _Item__price.

It will work fine.

Shahbaz A.
  • 4,047
  • 4
  • 34
  • 55