Is it better practice to have void method throw an exception or to have the method return a boolean?

Question

This falls straight into the same category with the recent "Is it better practice to return if false or execute an if statement if true?" question.

Often, while writing code, I find myself presented with two options. Either I can do this:

public void myMethod() throws Exception {
    // Some code
    validate(some_object_or_condition);
    // Some code
}

private void validate(Object arg) throws Exception {
    // Code that validates the argument and throws an exception if it's invalid
}

Or this:

public void myMethod() throws Exception {
    // Some code
    if( !isValid(some_object_or_condition) ) throw new Exception();
    // Some code
}

private boolean isValid(Object arg) {
    // Code that returns a boolean based on whether or not the argument is valid
}

Is either one preferable over the other? When I was still just learning Java the latter felt more "object oriented" and somehow more natural. However, with experience the former has started to feel better because then the code looks cleaner to me, but I'm afraid people might find it less readable.

Edit/Add: This time I ran into this problem when implementing a game (just for the fun of it) where the user would place words into a grid. The playWord method may fail because of the word not actually being a real word (yielding an InvalidWordException) or because the user is trying to place a valid word into an illegal position (yielding a WordPlacementException). If the method does not fail, it should return the score of the word as an integer. So in my case the playWord method first calls the method that validates the word and later the method that validates the location of the word, and these provoked this question here.

Answer 1

There's quite a few questions about this on stackoverflow but I think my general rule tends to be leave exceptions for exceptional circumstances and booleans for where failure occurances is an accepted/known possible behaviour.

So I personally probably prefer your option 2 (interested to see what others think), where if anything the exception is thrown a bit higher up the chain. That way I could re-use the isValid logic elsewhere and deal with the failure without needing to wrap it in an exception handler.

Of course, I think this is probably a case by case basis and depends on the kind of logic being checked.

I'm sure I remember a really good Q&A on this somewhere but can't quite find it. However found a couple of discussions:

Throw exception or return boolean - SO

Throwing exceptions - SO

EDIT: To further comment after the edits, perhaps you could also consider the option of returning status codes (enums) instead of the exceptions if there are a number of options. This has the benefit of being more efficient however as some have mentioned on SO could expose unecessary workings if this is a API or assembly. Some links I found on SO about this with some good discussions were:

Exception or error code enum to use error codes or not

And there were more. I see this coming down to a few options.

1. Throw exception
2. Return true/false
3. Return a status/error code enum

All probably have their place. Exceptions do occur the most overhead so if performance is an issue or the routine is called in many places perhaps this is not the best solution. I also saw mention that it was not necessarily a good idea to use exceptions to control the flow of application logic which it would seem to be doing in your case?

Basically my thoughts are to use return true/false, use a status error code, and then exceptions in that order.

An alternative suggestion might be something along the lines of (whether it's a decent alternative or not hopefully someone might comment on ???):

public int playWord(string w) {
    Word word = new Word(word);
    if (word.isValid()) {
       return word.getScore();
    } else {
       throw new InvalidDataException(word.GetErrorMessage());
    }
}

class Word {
   private String word;
   private String errorMessage = null;

   public Word(string w) {
       word = w;
   }

   public boolean IsValid() {
       // TODO: perform various checks calling private methods as required and if fail store against errorMessage
      return errorMessage == null;
   }

   public String GetErrorMessage() {
      return errorMessage;
   }

   public int getScore(string word) {
     // return the score of the word no matter what it is
   }
}

Answer 2

Returning a Boolean seems more natural to me. It gives you the option to handle this condition in different ways, depending on the situation. The IsValid method has only one very specific task, which is to determine if the object is valid. This follows SRP, the single responsibility principle, which states that a code entity (class, method, component) should have only one responsibility.

if(IsValid(obj))
    Save(obj);
else
    Messagebox.Show("Please enter a valid code");

In another context the same IsValid method can be reused without any changes

if(!IsValid(obj))
    throw new WhateverException();

Note: Wikipedia defines SRP on classes; however this principle can be applied to different levels of granularity.

Answer 3

The answer is it depends, or both.

1. If you are the user of that method, and know this is an infrequent occurrence, throw exceptions.
2. If you are user of the method, and know it is a frequent error, use booleans.
3. If you don't know the frequence because you are not the only user, have a Try pattern. I.e.

public void MyMethod throws ArgumentException { }

And

public bool TryMyMethod
{
}

Answer 4

Return the boolean (or better, return a meaningful object as suggested by Michael Anderson). If the validation failure is invalid, the calling method can choose to throw an exception.

That way, isValid() can throw an exception if there is an error while validating (perhaps loading a dictionary fails). This separates normal flow from exception processing.

Answer 5

It is interesting that everyone is saying boolean. While booleans can be useful exceptions are extremely powerful and offer you a lot of flexibility. Booleans leaves a hole for people who aren't paying attention to not validate that the function worked. With exceptions a single function can have multiple reasons for failure and each exception will explain that failure. Sometimes one failure is acceptable while another is not. This way you can catch one failure and just throw the others up the chain.

public void validate(String top, String bottom) {
  if(top == "") {
    throw EmptyTopException("You forgot to put something on the top");
  }
  if(bottom == "") {
    throw EmptyBottomException("You forgot to put something on the bottom");
  }
}

public void main(String args[]) {
  try {
    validate("spire", "foundation");
  }
  catch(EmptyTopException ex) {
     System.out.println("The top was empty we will have a shorter tower");
  }
  catch(EmptyBottomException ex) {
     throw new Exception("There was no foundation this tower is not physically possible", ex);
  }
}

It can get a little big with a lot of try catches but as you can see you can fail for two different reasons here. one which doesn't really make a difference and another that will tell the user that the input can not be accepted.

Answer 6

There's another option that is used more frequently in some other languages, which is to return the exception. This makes some sense when failure is not really exceptional and can be ignored in some cases.

public void myMethod() throws Exception {
    // Some code
    Exception e = validate(some_object_or_condition);
    if(e!=null) throw e;
    // Some code
}

private Exception validate(Object arg) {
   if(...) { 
       return new Exception(...);
   }
   ...
   return null;      
}

Now admittedly in many cases this looks pretty cumbersome, but where it really shines is when you start to need inversion of control or callbacks (where you pass the return values and exceptions to the callbacks rather than return them directly.)

When inversion of control is used this often looks like

public void myMethod() {
    Validator validator = getValidator();
    ErrorHandler eh = getErrorHandler();
    validator.validate(some_object_or_condition, eh);
}

public abstract class ErrorHandler {
  public void process(Exception e);
}

public void myMethod() throws Exception {
    // Some code
    Exception e = validate(some_object_or_condition);
    if(e!=null) throw e;
    // Some code
}

class Validator 
   private void validate(Object arg, XYZ xyz) {
   if(...) {
       xyz.process( new Exception(...) );
       return;
   }
   ...
   xyz.process(null);      
}

Again this can be uglier than the pure throw, or boolean return style, but in some cases it can result in more testable and code that can work better in a multi-threaded environment.