views:

211

answers:

10

I am often in a situation where I have a method where something can go wrong but an exception would not be right to use because it is not exceptional.

For example:

I am designing a monopoly game. The class Bank has a method buyHouse and a field which counts the number of houses left(there is 32 houses in monopoly). Something that could go wrong is a player buying a house when there is 0 left. How should I handle this. Here is 3 approaches I can come up with.

1. public void buyHouse(Player player, PropertyValue propertyValue)
{
    if(houseCount < 0) throw new someException;
    ....
    //Not really an exceptional situation
}

2. public boolean buyHouse(Player player, PropertyValue propertyValue)
{
    if(houseCount < 0) return false;
    ....
    //This I think is the most normal approach but changing something
    //and returning if it was a success seems bad practice to me.
}

3. public boolean housesLeft()
{
    if(houseCount > 0) return true;

    return false;

    //Introducing a new method. But now I expect the client to call this method
    //first before calling buyHouse(). 
}

What would you do?

+12  A: 

I would do 3 and 1 together. The proper usage of the API is to check if there are houses left before buying one. If, however, the developer forgot to do so, then throw a runtime exception.

If it is a multi-threaded situation (where many people are buying a house simultaneously) it gets more complicated. In that case, I would indeed consider a checked exception, if not a tryToBuyAHouse method that returns a boolean, but a runtime exception on the buyHouse method.

Yishai
I was going to say this, too. I think that an IllegalStateException is appropriate.
Paul Hanbury
Thanks - this seems to be the right choice.
bobjink
+4  A: 

I find the meaning of "exceptional" to be quite subjective. It means whatever you want it to mean. You're designing the interface to the function, you get to decide what is exceptional and what is not.

If you don't expect buyHouse to be invoked when houseCount is <= 0, then an exception here is fine. Even if you do expect it to be invoked, you can catch the exception in the caller to handle that situation.

Ned Batchelder
+2  A: 

Either (1) or (2) is acceptable, depending on whether or not you consider "no houses to buy" a routine result or an exceptional condition.

(3) is a bad idea, for several reasons:

  • it breaks encapsulation (client has to know too much about internals of the Bank)
  • you'd still have to check for errors and do (1) or (2) in case the client screws up
  • it's problematic in multi-threaded situations
David Gelhar
I don't see how 2 is different than three, except that even worse - if you forget to check, you can think that you bought the house when you didn't.
Yishai
@David I don't think that it is inappropriate to expect the client to know, and play in accordance with, the rules of the game.
Paul Hanbury
+1  A: 

I would do something like this:

public boolean BuyHouse(Player player, PropertyValue propertyValue) {
      // Get houseCount
      if(houseCount <= 0) {
       // Log this to your message queue that you want to show 
       // to the user (if it has a UI)
       return false;
      }
      // Do other stuff if houses are left
}

PS: I am not familiar with java, I use C#

Mahesh Velaga
+1  A: 

This question is hard to answer without the context of which entity has-a house. From a general design perspective, there is little semantic difference for the caller between (1) and (2) - both are try and check - but you are correct that (1) is to be shunned for wholly expectable state.

msw
+3  A: 

If something works as expected 32 times in a row, and then fails to function as expected, I think that you could justify making it an exceptional condition if it was an isolated case.

Given the situation you describe, I think that using exceptions is not appropriate, as once 32 houses are sold the bank will continue to be out of them (this is the new "normal" state), and exception processing is actually pretty slow in Java compared to normal processing.

One thing you could do is more closely mirror the actual interaction. In Monopoly, the banker will simply tell you that you cannot buy a house if there are none left.

A potential model for this is as follows:

public House buy(Player player, PropertyValue propertyValue) {
  House propertyHouse = null;
  if (houseCount > 0) {
    propertyHouse = new House(player, propertyValue);
    houseCount--;
  }

  return propertyHouse;
}

That would also allow you to add behavior to the House object, and make the flow for requesting/buying a house a little more natural. If there are no houses available, you don't get one.

mlschechter
I think this goes in the right direction, but I would prefer an Option type (see e.g. http://functionaljava.org ) to enforce that you check the result, avoiding NPEs
Landei
That's an interesting direction, and I can see some advantages there. Have you seen significant use of this type of paradigm in everyday programming, or is this more of an academic/research tool?
mlschechter
+1  A: 

You decide a rule & exception here for user who use your API/methods:

housesLeft() can be called to check the number of houses left before buyHouse() is called. Calling buyHouse() whenever the number of house left is zero is an exception.

It is similar to checking before you access certain array element, you check the array length before you try t o access it, else an exception will be issue.

So it should looks like this:

if (housesLeft() > 0) buyHouse(...);

similar to

for (int i=0; i < arrayList.length; i++) System.out.println(arrayList[i]);
ttchong
This pattern is prone to multi-threading issues.
Samit G.
@samG: yes, it is simple but can combine with other threading related pattern. I think multi-threading is not a concern for bobjink here.
ttchong
I think Yishai has post a good suggestion above for multi-threaded related case.
ttchong
+2  A: 

A couple of other options:

  • Your method could accept a number of houses requested parameter, and return the number of houses actually purchased, after checking the player's available balance and the number of houses available. Returning zero would be a perfectly acceptable possibility. You rely on calling code to check how many houses it actually got back, of course. (this is a variation on returning a boolean, where true/false indicate 1 or zero houses purchased, of course)

  • A variation on that theme would be to return a collection of House objects corresponding to the number of houses successfully purchased, which could be an empty collection. Presumably calling code would then be unable to act as if it had more House objects than you'd given it. (this is a variation on returning a House object, with null representing no houses purchased, and an object representing 1 house, and is often part of a general coding approach of preferring empty collections to null references)

  • Your method could return a HousePurchaseTransaction object which was itself queryable to determine the success or failure of the transaction, its actual cost, and so on.

  • A richer variation on that theme might be to make HousePurchaseTransaction abstract and derive two child classes: SuccessfulHousePurchase and FailedHousePurchase, so you could associate different behaviour with the two result conditions. Installing a house into a street might require you to pass a 'SuccessfulHousePurchase' object in order to proceed. (this avoids the danger of returning a null being the root of a later null reference error, and is a variant on the null object pattern)

In reality, I suspect the approach taken would depend on where you ended up allocating responsibility for placing the houses on the board, upgrading to hotels, enforcing even-build rules, limiting the number of houses purchased on a given street, and so on.

James Hart
Some good suggestion I had not thought of :)
bobjink
+5  A: 

This is very similar to the idea of popping an item off of an empty stack... it is exceptional. You are doing something that should fail.

Think of exceptional situations as cases where you want to notify the programmer that something has gone wrong and you do not want them to ignore it. Using a simple boolean return value isn't "right" since the programmer can just ignore it. Also the idea of having a method that should be called to check that there are houses available is a good idea. But remember that programmers will, in some cases, forget to call it. In that case the exception serves to remind them that they need to call the method to check that a house exists before acquiring it.

So, I would provide the method to check that there are houses, and expect that people will call it and use the true/false return value. In the event that they do not call that method, or ignore the return value, I would throw an exception so that the game does not get put into a bad state.

TofuBeer
+1  A: 

Remember that you can use

return houseCount > 0;

rather than

if(houseCount > 0) return true;

return false;
Dijkstra
I know but I prefer my way :)
bobjink