There are three (idiomatic) to handle functions that can fail during runtime.
1. Return boolean
The first one is to return boolean. This style is widely used in C and C-style APIs, like PHP. This leads - for example in PHP - often to code like this:
if (xyz_parse($data) === FALSE)
$error = xyz_last_error();
The downsides to this are obvious, this is why it has fallen out of fashion more and more, especially in OOP languages and languages with exception-handling.
2. Use an intermediary/state/result object
If you do not use exceptions, you still have the opportunity in OOP languages to return objects that describe the state.
For example, if you receive input from the user and want to validate the input, you know beforehand that you might get garbage and that the result will quite often not validate, and you might write code like this:
ValidationResult result = parser.validate(data);
if (result.isValid())
// business logic
else
error = result.getvalidationError();
3. Use exceptions
Java does this as you have shown with sockets. The reasoning is that creating a connection should succeed and only fails in exceptional circumstances which need special handling.
Applying it
In your case, I would just throw the exception directly, and even drop the helper method.Your helper method is badly named. It doesn't actually query whether the service is up (as the name suggests), but instead simply connects.
Use (checked) exceptions
Lets assume that connecting is what your method actually does. In this case, we name it more aptly connectToService and make it throw the exception directly:
public void connectToService() thows IOException {
// yada yada some init stuff, whatever
socket.connect();
}
public void proceedWhenError() {
try {
connectToService();
} else {
logger.debug("Exception happened, but it's alright.", exception)
// do stuff
}
}
public void doNotProceedWhenError() throws IllegalStateException {
try {
connectToService();
// do stuff
}
catch(IOException e) {
throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", exception)
}
}
Use boolean
On the other hand, it might be possible that your serviceUp (which is still badly named and would better be called isServiceUp) does actually query whether the service is running or not (which might have been started by someone else). In this case, using a boolean would be the proper way
public boolean isServiceUp {
return ...; // query the service however you need
}
This, of course, refers to the first solution of using booleans and isn't really helpful when you also need to know why the service isn't up.
Use intermediary/result/state objects
So lets dismiss this limited approach and rework it using an intermediary/result/state object:
class ServiceState {
enum State { CONNECTED, WAITING, CLOSED; }
State state;
IOException exception;
String additionalMessage;
public ServiceState (State state, IOException ex, String msg) {
[...] // ommitted for brevity
}
// getters ommitted for brevity
}
The helper function would become getServiceState:
public ServiceState getServiceState() {
// query the service state however you need & return
}
public void proceedWhenError() {
ServiceState state = getServiceState();
if (state.getState() == State.CONNECTED) {
// do stuff
} else {
logger.debug("Exception happened, but it's alright.", state.getException())
// do stuff
}
}
public void doNotProceedWhenError() {
ServiceState state = getServiceState();
if (state.getState() == State.CONNECTED) {
// do stuff
} else {
// do stuff
throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", exception)
}
}
Note that you are also repeating yourself in proceedWhenError, the whole code could be simplified to:
public void proceedWhenError() {
ServiceState state = getServiceState();
if (state.getState() != State.CONNECTED) {
logger.debug("Exception happened, but it's alright.", state.getException())
}
// do stuff
}
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 second option. 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 second option when its very unclear whether the call should succeed (like validating data). But there are different opinions on this.
It also depends on what your helper method actually does. isServiceUp implies that it queries some state, but doesn't change it. In that case, returning an state object is obviously the idiomatic way to handle this.
But your implementation shows that the helper method connects to the service. In which case, at least in java, the idiomatic way to handle it would be to throw a (checked) exception - but you could also still justify using a result object.
Simply returning boolean is not advisable, though, since it is too limited. If all you need to know whether the service is running or not (and aren't interested in the cause), such a method might still be useful, though (and could, behind the scenes, be implemented as helper method that just does return getServiceState() == State.SUCCESS).
It is an anti-pattern to use Exceptions to control the workflow
So, lets see, what is an exception?
Definition: An exception is an event, which occurs during the execution of a program, that disrupts the normal flow of the program's instructions.
Source: https://docs.oracle.com/javase/tutorial/essential/exceptions/definition.html
The very definition of an exception is that it is an event that disprupts (and thus changes) the workflow of the program. If using it as defined is an anti-pattern, then the whole language feature in itself must be considered an anti-pattern. There are people that do believe exceptions are bad, and some have some valid arguments for it, but in general, exceptions are regarded as useful constructs and seen as useful tools to handle exceptional workflows.
Throwing an exception is certainly not an anti-pattern. Returning it, on the other hand, is. Thus yes, your code - as presented - is non-idiomatic.
