Is “assert false;” a good practice?

2019-07-08 09:42发布

问题:

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?

回答1:

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



回答2:

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.



回答3:

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.



回答4:

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.