I am currently writing a function returning an object from a list according to a given criteria. It looks like this:
for (Object object : list) {
if (condition(object)) {
return object;
}
}
This function should always return something from the list, and in the case no matching object was found, it was a bad call, a critical error, and the program should stop.
So, as I work with assertions enabled, I did the following just after the loop:
assert false; // Will always trigger in debug mode.
return null; // No matter anyway, an AssertionException has already been thrown.
But I wonder if I did well or not?
If not, what should I do instead? Throws an exception myself?
In any case, is there any kind of norm about this situation?
I would rather try to check the return value of the function when calling it.
if (yourFunctionWithList(parameter) == null)
//error handling, maybe throw new NPException or whatever.
else
//some object was returned
You may also write your own Exception class and handle it in any way you want.
I personally do not think assert false
is good practice.
EDIT
If it is about the AssertionException
that will be thrown, then you could also use it like
throw new AssertionError ("your error msg here");
So you can handle it the same way
The problem with asserting or throwing an exception is that you need to use exception handling to handle something which is not really an exceptional case.
Moreover, you can't really be sure what threw the exception/assertion that you caught. It might be the thrown at the place you expect, but it could alternatively have been thrown in the filtering code, for instance - so checking for an exception to detect the "not found" case may conflate other problems with that case
An alternative is to use Optional
(a similar class exists in Guava for pre-Java 8; or you can simply use a Set<Object>
, but that doesn't convey that you expect exactly 0 or 1 values to be found):
Optional<Object> method(List<?> list) {
for (Object object : list) {
if (condition(object)) {
return Optional.of(object);
}
}
return Optional.empty();
}
Now, in your calling code, you explicitly know that you may not have an item found in the list:
Optional<Object> opt = method(list);
if (opt.isPresent()) {
Object obj = opt.get();
// Handle the fact it was found.
} else {
// Handle the fact it wasn't found.
}
rather than exception handling, which you may forget to add.
I personally don't trust them; they're often disabled by default, even in development environment. That can make them give a very false sense of safety - both static analysis tools and other programmers can mistakenly assume they work as expected.
You should throw an Exception, which is the proper way in java:
for (Object object : list) {
if (condition(object)) {
return object;
}
}
throw new Exception('Failed: no matching item found!');
If you change your mind one day and dont want the program to stop, you will be able to catch the exception.