Is this setter 'evil'

Question

There's alot of talk about getters and setters being 'evil' and what not.

My question is: is the following setter evil? (rest of class omitted for brevity's sake)

int balance

public void deposit(int amount)  
{  
    this.balance += amount;  
}

This class is emulating an ATM. In the UK there are a few ATM's that lets you deposit as well as withdraw therefore this object needs a way of changing its state (the balance). Is this setter 'evil'?

Answer 1

Except for the fact that there is no handling of exceptional conditions, it looks like a perfectly good OO method - it's called what it does, and it does what you'd expect.

Answer 2

For a class, there's nothing evil about setting a value via a setter, but that's more of a function than a direct setter. Yes, it sets the value of a property, but it does it via addition rather than replacing the previous value and the names don't line up.

A real 'setter' would look more like this:

int balance

private void setBalance(int amount)
{
    this.balance = amount;
}

public void deposit(int amount)  
{  
    setBalance(this.balance + amount);  
}

For your specific ATM problem, though, I very much doubt that an ATM adds a deposit to your balance immediately. It likely needs to be collected and posted via a separate mechanism.

Answer 3

Well you would want to check for negative amounts, a zero amount, etc... but give the requirement it is ok.

Follow this rule of thumb, every variable you make should be final unless it has to change and never make set methods for instance variables unless you really want them to be changed outside of the class.

Answer 4

Not necessarily; you mention that you want to emulate the behaviour of an ATM (cash machine). And you're concerned that ATMs let you deposit as well as withdraw. But those operations, the deposit and withdrawl, would have to be serialized. You need all of your actions to be atomic, so this sort of method is better than one where you try to do more things.

Answer 5

I don't believe that that is what is meant when people talk about getters and setters, because this is not simply setting a member to the given value.

I don't care for setters and getters, but mostly because I think of my "objects" as higher-level entities in the codebase. E.g. (IMO) it would be "more wrong" to do the operation outside of the class:

account.SetBalance(account.GetBalance() + depositAmount)

Instead, you've implemented higher-level functionality in your object; you make a deposit and let the object figure out the right way of dealing with it. This allows much more centralized handling of exceptional conditions than the getter/setter example I gave above.

Answer 6

One issue that I see is that you are using an integral type when dealing with money. Not an issue if this is a fixed-point number but there is no indication that it is so.

Answer 7

Personally, I would call that a method, not a setter. The stereotypical setter would be

public void deposit(int new_balance)
{
    this.balance = new_balance;
}

All it does is give you direct access to the internals of the class, thus defeating any value gained by encapsulating them and restricting access. Which is why people don't like them.

Answer 8

IMO, the ATM should not have 'balance' as a field.

(additionally, your 'deposit' method is not a setter)

You should probably have an Account object with a 'balance' field and possibly a convenience method 'modifyBalance' on it that takes a positive value to increment or a negative value to decrement the balance.

Then your ATM methods would call 'modifyBalance' on the Account object when performing those types of transactions.

Answer 9

You can't tell whether a single method is evil or not, it depends on the context and who has access to the object.

If you have getters and setters for all fields and everybody and his dog have access to the object, then that is very bad, as there is essentially no encapsulation of the data.

If on the other hand you have setters only for the fields which need it and the object is only known to a select few other objects which need to communicate with it, then that would be quite OK.

Answer 10

Is that a trick question? I ask because the provided method isn't even a "setter" method. It's an operation, not a property. Setters and Getters are generally accessor methods for private variables (properties). So i guess the answer to your question is:

That's not a setter, but as a general method that performs an operation on an object, it's not evil at all.

Answer 11

That's not a setter. That's a normal method (or member function, or whatever).

A setter is a function that sets a given variable to a given value, and is usually a bad idea. A method is a function that performs a given class operation. It's meaningful in terms of the class.

If you have a weird data structure, you may not actually have a "balance" variable. No matter what your data structure, you're going to have to have a "deposit" function. There's part of the difference.

Answer 12

that's not a setter, its a normal method

even if it was a setter, it's not evil

this is an evil setter

int _balance = 0;  
public int Balance()  
{  
    get { return _balance; }  
    set { }    //now that's evil!  
}