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 they 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 failure modes.
- Return true/false
- Use
void
, throw (checked) exception
- Return a intermediate result object (or a general purpose ADT for this use-case, like
Result<S, E> = Success<S> | Fail<E>
, if your language supports this. Java e.g. will be able to do this once we get sealed types & records).
Return true
/false
This is used in some older, mostly c-style APIs. The downsides are obvious, 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.
Using an ADT
With an ADT, the above code would look like this (yes, even in java, once sealed types & full type patters for switch are implemented):
var result = parser.parse(data);
switch (result) {
case Success (MyDocument document) -> process(document);
case Fail (ValidationError error) -> showError(error);
}
Which also has a nice logic and a nice control flow to it.
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 is fine. But we have checked exceptions in Java, so I see no reason not to use them. I use checked exceptions when the basic assumption is that the call should succeed (like using a socket), but failure is possible, and I use the third option when its very unclear whether the call should succeed (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 succeed, and when it does not, that is 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.
Result<T, E>
whereT
is the result if successful andE
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.fillInStackTrace();
is called in the super constructor, in the classThrowable