29

To simplify the interface, is it better to just not have the getBalance() method? Passing 0 to the charge(float c); will give the same result:

public class Client {
    private float bal;
    float getBalance() { return bal; }
    float charge(float c) {
        bal -= c;
        return bal;
    }
}

Maybe make a note in javadoc? Or, just leave it to the class user to figure out how to get the balance?

gnat
  • 20,543
  • 29
  • 115
  • 306
david.t
  • 437

6 Answers6

89

You seem to suggest that the complexity of an interface is measured by the number of elements it has (methods, in this case). Many would argue that having to remember that the charge method can be used to return the balance of a Client adds much more complexity than having the extra element of the getBalance method. Making things more explicit is much simpler, especially to the point where it leaves no ambiguity, regardless of the higher number of elements in the interface.

Besides, calling charge(0) violates the principle of least astonishment, also known as the WTFs per minute metric (from Clean Code, image below), making it hard for new members of the team (or current ones, after a while away from the code) until they understand that the call is actually used to get the balance. Think of how other readers would react:

enter image description here

Also, the signature of the charge method goes against the guidelines of doing one and only one thing and command-query separation, because it causes the object to change its state while also returning a new value.

All in all, I believe that the simplest interface in this case would be:

public class Client {
  private float bal;
  float getBalance() { return bal; }
  void charge(float c) { bal -= c; }
}
Community
  • 1
MichelHenrich
  • 6,245
28

IMO, replacing getBalance() with charge(0) across your application isn't a simplification. Yes it is fewer lines, but it obfuscates the meaning of the charge() method, which could potentially cause headaches down the line when you or someone else needs to revisit this code.

Although they might give the same result, getting the balance of an account is not the same as a charge of zero, so it would probably be best to seperate your concerns. For example, if you ever needed to modify charge() to log whenever there is an account transaction, you now have a problem and would need to separate the functionality anyway.

Matt Dalzell
  • 381
13

It's important to remember that your code should be self-documenting. When I call charge(x), I expect x to be charged. Information about balance is secondary. What's more, I may not know how charge() is implemented when I call it and I definitely won't know how it's implemented tomorrow. For example, consider this potential future update to charge():

float charge(float c) {
    lockDownUserAccountUntilChargeClears();
    bal -= c;
    Unlock();
    return bal;
}

All of a sudden using charge() to get the balance doesn't look so good.

Peter Mortensen
  • 1,045
David says Reinstate Monica
  • 2,789
2

Using charge(0); to get the balance is a bad idea: one day, someone might add some code there to log the charges being made without realising the other use of the function, and then every time someone gets the balance it will be logged as a charge. (There are ways around this such as a conditional statement that says something like:

if (c > 0) {
    // make and log charge
}
return bal;

but these rely on the programmer knowing to implement them, which he won't do if it isn't immediately obvious that they are necessary.

In short: don't rely on either your users or your programmer successors realising that charge(0); is the correct way to get the balance, because unless there's documentation that they are guaranteed to not miss then frankly that looks like the most frightening way of getting the balance possible.

micheal65536
  • 209
0

I know there are plenty of answers, but another reason against charge(0) is that a simple typo of charge(9) will cause your customer's balance to diminish every time you want to get their balance. If you have good unit testing you might be able to mitigate that risk, but if you fail to be diligent on every call to charge you could have this mishap.

Shaz
  • 2,612
-2

I'd like to mention a particular case where it would make sense to have fewer, more multipurpose, methods: if there is a lot of polymorphism, that is, many implementations of this interface; especially if those implementations are in separately developed code that can't be updated in sync (the interface is defined by a library).

In that case, simplifying the job of writing each implementation is much more valuable than the clarity of usage of it, because the former avoids contract violation bugs (the two methods being inconsistent with each other), whereas the latter only hurts readability, which can be recovered by a helper function or superclass method defining getBalance in terms of charge.

(This is a design pattern, which I do not recall a specific name for: defining a complex caller-friendly interface in terms of an minimal implementor-friendly one. In Classic Mac OS the minimal interface for drawing operations was called the "bottleneck" but this does not seem to be a popular term.)

If this is not the case (there are few implementations or exactly one) then separating the methods for clarity, and to allow the simple addition of behavior relevant to nonzero charges to charge(), makes sense.

Kevin Reid
  • 166