3

What is the preferred design for methods that encounter predictable but unpreventable failure? Should they return an empty result or throw an exception on failure? Please explain your reasoning.

I'll give two sample methods:

  1. A method like Arrays.binarySearch() that returns the index of a value within an array.
  2. Tickets.buy(20, 100). A ticket bot attempts to buy at least 20 tickets using $100. The method returns the list of tickets that were bought.

For both methods, let us assume that the user has no way of predicting whether the method will succeed in finding a match.

Method 1 can fail if:

  • The array does not contain the desired value.

Method 2 can fail if:

  • There aren't enough tickets.
  • There are enough tickets for sale, but they cost more than $100.

Java chose to return the equivalent to an empty result for method 1, but doing the same for method 2 wouldn't allow you to differentiate between the two kinds of errors. What are the guidelines for choosing between the two possible designs?

Please note that although this question contains two concrete methods, I am looking for general design guidelines not a specific solution to the aforementioned methods.

Gili
  • 86,244
  • 97
  • 390
  • 689
  • Maybe you could return different error codes for the second one – Thiyagu May 03 '18 at 17:13
  • @user7 Doing so would prevent me from returning the list of tickets on success. Also, please note that I am looking for general guidelines as opposed to a solution to the specific examples I gave. – Gili May 03 '18 at 17:14
  • Valid.. Or, you can return a list in normal operation and can throw an exception that has the *reason* (enum maybe) as an instance variable with proper getters. (Whether an exception is valid here is something that is debatable) – Thiyagu May 03 '18 at 17:16
  • Regarding whether an exception makes sense, I rather like the pragmatism of the accepted answer [here](https://softwareengineering.stackexchange.com/q/184654/7866): try implementing the same thing with Exceptions versus return types, and see which code comes out cleaner and easier to understand. – StriplingWarrior May 03 '18 at 19:52
  • 1
    @StriplingWarrior Thank you for pointing me in this direction. I read the accepted answer and many others below it but ultimately I found https://stackoverflow.com/a/77361/14731 to be far more satisfying (and less subjective). – Gili May 04 '18 at 05:19
  • That's a really good answer. Thanks for linking to it. So if your application is calling `buy()` because it's already made a good-faith effort to determine that the tickets are available for purchase, and it turns out that they aren't after all, it makes sense to throw an exception. I wouldn't recommend using `buy()` as a way to check if the tickets are really available in the first place, though. Most online ticketing systems create temporary reservations while the user walks through the purchasing screens, so cases where the tickets aren't available when they click "buy" are exceptional. – StriplingWarrior May 04 '18 at 15:20

4 Answers4

1

The best approach depends on the case.

Case 1: Since the method returns the index of the found element, I suggest returning -1 if the desired value has not been found. Stick to the well known API of almost everyone is familiar with, like for indexOf().

Case 2: Here I would suggest using exceptions (InsufficientMoneyException, OutOfTicketsException, ...). An empty list or null is not very meaningful and the different error cases cannot be reflected with that. This way you can properly handle the exceptions in their respective catch blocks.

In general you have to keep in mind that throwing exceptions is expensive operations, because the stack trace has to be built.

Felix
  • 959
  • 5
  • 13
  • Thank you for your suggestion. Although the question contains specific examples I am actually looking for general guidelines. In the aforementioned case, **why** would you return `-1` for case 1 versus throwing exceptions for case 2? Forget for a moment that case 1 is a known API method because that is not intentional. – Gili May 03 '18 at 17:32
  • For **Case 1** there are 2 possible results: `-1` for "not found" and `>=0` for the found index. **Case 2** is a more complex operation having a larger set of possible results. The return type of that method is not able to transfer all the execution results. – Felix May 03 '18 at 17:41
  • I think you hit ENTER too soon. You wrote that there are 2 possible results, but you only listed one: `-1`. – Gili May 03 '18 at 17:43
  • Right, so if I understand you correctly you are saying that the number of error classes determines whether one should use an empty result vs exceptions. Is that correct? Would you still return `-1` for case 1 if it wasn't a known API or would you consider throwing an exception there too? – Gili May 03 '18 at 17:51
  • Note that the expense of throwing an exception is pretty relative. In most applications, the I/O operations involved in something like searching for available tickets are going to vastly outweigh the performance implications. On the other hand, the fact that the Exception framework is collecting stack traces should be an indication that it's intended for situations where you need to track down bugs in your code--not for things you'd expect to happen hundreds of times a day. – StriplingWarrior May 03 '18 at 19:49
1

This question is rather opinion-based, just like the whole returning null/throwing an exception/returning empty object dilemma.

I personally think that returning -1 is for the first method and throwing exceptions for your second method would be better.

Following the single responsibility principle, one method should do one thing.

The goal of the first method is to search for an element, it can either find it or not find it. This is the expected behavior.

The goal of the second method is to buy a ticket. Unlike the first method, it's not checking if it can buy a ticket or not, it's doing it. The expected behavior is that the ticket is bought. If that behavior does not happen, something went wrong, thus an exception should be thrown.

I also think that @StriplingWarrior's approach might not be ideal because if you have more than just two possible cases, you can't really have Either<A, B, C, D, E, ...>.

RIP. I spent 15 minutes writing an answer and I accidentally opened the web console and deleted the body. The last saved answer was like 10 minutes before. I don't even know how you can accidentally do that. I guess F12 and DEL are kinda close

TwiN
  • 3,554
  • 1
  • 20
  • 31
  • The `Either<,>` approach handles three possible cases, but only requires two return types because a variety of possible reasons for failure can be represented by a `Left` value, whereas you only expect to get a `List` when things succeed. You won't typically find `Either<>` implementations with more than two type arguments. Regarding SRP, perhaps the method should be renamed to something like `searchForTicketsToBuy()` because, just like `binarySearch()` there's an expectation that it'll either find tickets that match the given parameters or it won't. – StriplingWarrior May 03 '18 at 19:37
0

Typically you'd want to make your return type be an object that contains all the information you're likely to need to know.

In the early days of Java, it was common to return normally invalid values like -1 (in the case of Arrays.binarySearch()) or null in order to represent the absence of a value. More modern APIs tend to return an Optional<> to provide compile-time enforcement of the concept that there may not be a result (as well as differentiating between "no result" and "the result is null).

In the Tickets.buy() example, you could have an Either<String, List<Ticket>>, where the "left" value can be a reason that the purchase failed, and the "right" value can be the tickets that got purchased if it succeeded.

StriplingWarrior
  • 151,543
  • 27
  • 246
  • 315
  • I'm divided. On the one hand, I was taught that we should use exceptions for all exceptional cases (which goes against `Either>`). On the other hand, I'm sure there are reasons against throwing an exception for `Arrays.binarySearch()` (hopefully they are something other than performance because that's a moving target). – Gili May 03 '18 at 17:36
  • [Exceptional](http://www.dictionary.com/browse/exceptional?r=75&src=ref&ch=dic) _noun_ forming an exception or rare instance; unusual; extraordinary. I think it makes sense to use Exceptions for _exceptional_ circumstances, meaning things you don't normally expect to happen. An Exception usually indicates that someone probably wrote their code the wrong way, or that there's some kind of hardware-level malfunction. Exceptions should be avoided for situations that are part of a normal logic flow in your application. – StriplingWarrior May 03 '18 at 19:25
  • Note the difference between returning -1 to represent no result versus the Exceptions thrown by [this overload of binarySearch](https://docs.oracle.com/javase/7/docs/api/java/util/Arrays.html#binarySearch(java.lang.Object[],%20int,%20int,%20java.lang.Object)). – StriplingWarrior May 03 '18 at 19:25
  • Your examples refer to predictable, preventable type of exceptions. There is a separate category of exceptions for predictable, unpreventable scenarios (e.g. you try reading a file that gets deleted between the time you check for its existence and the time the read operation begins). This question focuses exclusively with these kind of exceptions (predictable but unpreventable). – Gili May 04 '18 at 04:59
  • @Gili: I like the way you worded that distinction. If your application already checked for availability of those tickets and then they got purchased before the call to `buy()`, then that distinction applies well to your example. On the other hand, if you're just calling `buy()` and expecting to use the Exceptions as a check to see whether the tickets are available, I'd argue that doesn't fall under the "unpreventable" umbrella. In that case, it's part of your standard logic flow. – StriplingWarrior May 04 '18 at 15:14
0

I ended up taking the advice found at https://stackoverflow.com/a/77361/14731

My interpretation is as follows:

  • From a purist perspective, if the purpose of Arrays.binarySearch() is to return the index of an element in an array, then it should throw an exception when no match is found. The fact that no result has a negative index is a convenient fact that allows us to return -1 instead. But again, in an ideal world this method should be throwing an exception. If this method was answering the question "does the array contain this element" and returning a boolean then no exception would need to be thrown.
  • Tickets.buy(20, 100) assumes that a match will be found in purchase the tickets. If no match is found then an exception should be thrown.
Gili
  • 86,244
  • 97
  • 390
  • 689