Consider the following method:
private static String method (String string) {
if (string.equals("conditionOne")) {
return value;
} else if (string.equals("conditionTwo")) {
return symbol;
} else {
return null;
}
}
Let's say I am checking for two conditions, conditionOne
and conditionTwo
. Also, assume that some other part of the program ensures that only these two cases will ever happen. Since the method has to return something for all cases to avoid a compiler error, is it okay to return null
for the final else
block just for syntactical purposes since that part will never execute?
Edit: For clarity, I'd like to mention that the compiler gives me an error ("Expecting return statement") if I don't include that last else
block. Other than to returning null (or an empty string, as pointed out by Anthony below), is there another way to write this method so that this does not happen?
Thanks
You're describing a very common scenario while programming. You intend for a certain thing to never happen, but the compiler also knows it could happen. The proper way to handle such code paths is to ensure that they are never reached, generally by throwing an AssertionError
or a RuntimeException
such as IllegalArgumentException
, IllegalStateException
or UnsupportedOperationException
. This is referred to as failing-fast.
In your case I would throw an IllegalArgumentException
as that's clearly what's happened - your method expects exactly two inputs; anything else is forbidden and you should fail-fast in such cases. Effective Java Item 38 also discusses this.
private static String method (String condition) {
if (condition.equals("conditionOne")) {
return value;
} else if (condition.equals("conditionTwo")) {
return symbol;
}
throw new IllegalArgumentException("Invalid condition '" + condition +"'");
}
Now you can be confident the only inputs this function will support are the ones it is designed to support. Even better, anyone calling your method incorrectly will get a clear, actionable error message.
The Guava User Guide has a good overview of different kinds of failures and when you should raise them.
You could also avoid this issue other ways - namely by defining a better method signature. It looks like you're defining a "stringly-typed" API; using an enum would help prevent callers from passing in erroneous parameters. See also Effective Java Items 50 and 30.
In rare cases (generally when dealing directly with user input) you'll want to fail-soft rather than fail-fast. This is common with confirmation dialogs; if you ask the user to enter "Yes" or "No" it's generally sufficient to simply check whether they entered "Yes" - whether they entered "No" or "Uhhhh", you'll just treat it as not-"Yes".
As you have defined the function as returning a String, it would be more correct to have
if (string.equals("conditionOne")) {
return value;
} else if (string.equals("conditionTwo")) {
return symbol;
} else {
return "";
}
You can write something like this:
if (string.equals("conditionOne")) {
return value;
} else if (string.equals("conditionTwo")) {
return symbol;
}
return "";
or like this:
string rval = "";
if (string.equals("conditionOne")) {
rval = value;
} else if (string.equals("conditionTwo")) {
rval = symbol;
}
return rval;
or like this
if (string.equals("conditionOne")) {
return value;
} else if (string.equals("conditionTwo")) {
return symbol;
}
throw;
edited.
If you've already guaranteed that the string will always equal either conditionOne
or conditionTwo
, then writing the 3rd else
condition is not necessary.
It's equivalent to adding code that does not check if it's either conditionOne
or conditionTwo
. It's better off to remove it.
Edit Seeing your edit, I would recommend the above solution of returning ""
since it's a string.