22

I'm building an API, a function that uploads a file. This function will return nothing/void if the file was uploaded correctly and throws an exception when there was some problem.

Why an exception and not just false? Because inside an exception I can specify the reason of failure (no connection, missing filename, wrong password, missing file description, etc.). I wanted to build a custom exception (with some enum to help the API user to handle all the errors).

Is this a good practice or is it better returning an object (with a boolean inside, an optional error message and the enum for errors)?

  • 5
    Possible duplicate of Return magic value, throw exception or return false on failure? – gnat Sep 12 '16 at 7:33
  • 59
    True and false go well together. Void and exception go well together. True and exception go together like cats and taxes. Someday I may be reading your code. Please don't do this to me. – candied_orange Sep 12 '16 at 7:56
  • 4
    I quite like patterns where an object is returned that encapsulates either success or failure. For example, Rust has Result<T, E> where T is the result if successful and E is the error if unsuccessful. Throwing an exception (can be - not so sure in Java) can be expensive as it involves unwinding the call stack, but creating an Exception object is cheap. Obviously, stick to the established patterns of the language you're using, but don't combine booleans and exceptions like this. – Dan Pantry Sep 12 '16 at 13:09
  • 4
    Be careful here. You may be headed down a rabbit hole. When your program is capable of dealing with a problem, it's usually easier to just detect and deal with it by checking the preconditions. You would rarely catch an exception, examine its data in code, and try again after fixing the problem. As such, it's rarely valuable to have very many exception types. If you're trying to log problems, this makes a lot more sense, but don't expect to write a bunch of catch blocks with significant amounts of code in them. – jpmc26 Sep 12 '16 at 15:57
  • 4
    @DanPantry In Java, the call stack is checked when creating the exception, not when throwing it. fillInStackTrace(); is called in the super constructor, in the class Throwable – Simon Forsberg Sep 12 '16 at 19:15
34

Throwing an exception is simply an additional way of making a method return a value. The caller can check for a return value just as easily as catch an exception and check that. Therefore, deciding between throw and return requires other criteria.

Throwing exceptions should often be avoided if it endangers the efficiency of your program (constructing an exception object and unwinding the call stack is much more work for the computer than just pushing a value onto it). But if the purpose of your method is to upload a file, then the bottleneck is always going to be the network and file system I/O, so it's pointless to optimize the return method.

It's also a bad idea to throw exceptions for what should be a simple control flow (e.g. when a search succeeds by finding its value), because that violates the expectations of API users. But a method failing to fulfill its purpose is an exceptional case (or at least it should be), so I see no reason for not throwing an exception. And if you do that, you might just as well make it a custom, more informative exception (but it's a good idea to make it a subclass of a standard, more general exception like IOException).

  • 7
    If it is an expected outcome for a file upload request to fail, I would be surprised by an exception thrown in my face (especially if there is a return value in the method signature). – hoffmale Sep 12 '16 at 11:59
  • 1
    Note that the reason for making it extend a standard exception is that many (probably even most) callers won't write code to try to fix the problem. As a result, most callers will want a simple, "catch this big group of exceptions," without having to list them all explicitly. – jpmc26 Sep 12 '16 at 15:52
  • 4
    @gbjbaanb That's why exceptions should be used when there really is some unfixable situation. When you are trying to open a file and the file can't be read, this is a good case for an exception. When you check for file existence, a boolean needs to be used because it is well expected that the file may not be there. – Malcolm Sep 12 '16 at 17:40
  • 21
    Kilian is saying, in the recursive slogan "exceptions are for exceptional situations", exceptional situations are when the function cannot fulfil its purpose. If the function is supposed to read a file, and the file cannot be read that is exceptional. If the function is supposed to tell you whether or not a file exists (never mind the potential TOCTOU), then the file not existing is not exceptional because the function can still do what it's supposed to do. If the function is supposed to assert that a file exists, then it not existing is exceptional because that's what "assert" means. – Steve Jessop Sep 12 '16 at 17:56
  • 10
    Note that the only difference between a function that tells you whether a file exists, and a function that asserts that a file exists, is whether or not the case of the file not existing is exceptional. People sometimes speak as if it's a property of the error condition whether or not it "is exceptional". It is not a property of the error condition, it's a combined property of the error condition and the purpose of the function it occurs in. Otherwise we'd never catch exceptions. – Steve Jessop Sep 12 '16 at 17:56
29

There is absolutely no reason for returning true on success if you don't return false on failure. What should the client code look like?

if (result = tryMyAPICall()) {
    // business logic
}
else {
    // this will *never* happen anyways
}

In this case, the caller needs a try-catch block anyways, but then he can better write:

try {
    result = tryMyAPICall();
    // business logic
    // will only reach this line when no exception
    // no reason to use an if-condition
} catch (SomeException se) { }

So the true return value is completely irelevant for the caller. So just keep the method void.


In general, there are three ways to design failre modes.

  1. Return true/false
  2. Use void, throw (checked) exception
  3. return a intermediate result object.

Return true/false

This is used in some older, mostly c-style APIs. The downsides are obviuos, you have no idea what went wrong. PHP does this quite often, leading to code like this:

if (xyz_parse($data) === FALSE)
   $error = xyz_last_error();

In multi-threaded contexts, this is even worse.

Throw (checked) exceptions

This is a nice way to do it. At some points, you can expect failure. Java does this with sockets. The basic assumption is that a call should succeed, but everyone knows that certain operations might fail. Socket connections are among them. So the caller gets forced to handle the failure. its a nice design, because it makes sure the caller actually handles the failure, and gives the caller an elegant way to deal with the failure.

Return result object

This is another nice way to handle this. Its often used for parsing or simply things that need to get validated.

ValidationResult result = parser.validate(data);
if (result.isValid())
    // business logic
else
    error = result.getvalidationError();

Nice, clean logic for the caller as well.

There is a bit of debate when to use the second case, and when to use the third. Some people believe that exceptions should be exceptional and that you should not design with the possibility of exceptions in mind, and will pretty much always use the third options. That fine. But we have checked exceptions in Java, so I see no reason not to use them. I use checked expetions when the basic assumption is that the call should suceed (like using a socket), but failure is possible, and I use the third option when its very unclear wether the call should suceed (like validating data). But there are different opinions on this.

In your case, I would go with void + Exception. You expect the file upload to suceed, and when it doesn, its exceptional. But the caller is forced to handle that failure mode, and you can return an Exception that properly describes what kind of error occurred.

5

This really comes down to whether a failure is exceptional or expected.

If an error isn't something you generally expect, then it's reasonably likely the API user will just call your method without any special error handling. Throwing an exception allows that to bubble up the stack to a place where it gets noticed.

If on the other hand errors are commonplace, you should optimize for the developer who is checking for them, and try-catch clauses are a bit more cumbersome than an if/elif or switch series.

Always design an API in the mindset of someone who is using it.

  • 1
    In my case, like somebody pointed out, should be an exceptional failure that the API user can't upload the file. – Accollativo Sep 13 '16 at 8:24
4

Don't return a boolean if you never intend to return false. Just make the method void and document that it will throw an exception (IOException is suitable) on failure.

The reason for this is that if you do return a boolean, the user of your API may conclude that he/she can do this:

if (fileUpload(file) == false) {
    // Handle failure.
}

That will not work of course; that is, there is an incongruity between your method's contract and its behaviour. If you throw a checked exception, the user of the method has to handle failure:

try {
    fileUpload(file);
} catch (IOException e) {
    // Handle failure.
}
3

If your method has a return value, throwing an exception might surprise its users. If I see a method return a boolean, I am pretty convinced it will return true if it succeeds and false if it doesn't, and I will structure my code using an if-else clause. If your exception is going to be based on an enum anyway, you may as well return an enum value instead, similar to windows HResults, and keep an enum value for when the method succeeds.

It is also annoying to have a method throw exception when it is not the final step in a procedure. Having to write a try-catch and divert control flow to the catch block is a good recipe for spaghetti, and should be avoided.

If you are going forward with exceptions, try returning void instead, users will figure out it succeed if no exceptions are thrown, and none will try to use an if-else clause to divert control flow.

  • 1
    The enum was just an idea to help the API user to handle different types of errors without parsing the error message. So from your point of view in my case is better returning the enum and add an enum for "OK". – Accollativo Sep 12 '16 at 12:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.