Is "assert false;" a good practice?

Question

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?

Seems like a dupe of http://stackoverflow.com/questions/28480068/assertfalse-vs-runtimeexception, but I don't want to throw my unilateral dupe-hammer on it... — yshavit, Apr 07 '16 at 06:22
The context is a whole other, as well as the issue, I do not think you need to throw your hammer. +1 for making me laugh about the dupe-hammer at 8 o'clock am at work :D — Dominik Reinert, Apr 07 '16 at 06:24

Dominik Reinert · Accepted Answer · 2016-04-07T06:34:45.267

5

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

edited Apr 07 '16 at 06:34

answered Apr 07 '16 at 06:21

Dominik Reinert

1,075
3
12
26

But what if I call it from many pieces of code? Wouldn't it be simpler to check it once for all at the end of the function rather than each time I call it? – Aracthor Apr 07 '16 at 06:24
1

@yshavit right, haven't used them in a long time. Misinformation on my side, added an edit – Dominik Reinert Apr 07 '16 at 06:25
@Aracthor Especially if you call it many times you should use the syntax given to have more detailed opportunity of error handling. But maybe I don't get the comment-question false – Dominik Reinert Apr 07 '16 at 06:27
In my approach you could also handle the null by creating a dummy to work with further (in case you want a default-value) – Dominik Reinert Apr 07 '16 at 06:30

Andy Turner · Answer 2 · 2016-04-07T06:49:39.820

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.

score 1 · Answer 3 · answered Apr 07 '16 at 06:25

1

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.

answered Apr 07 '16 at 06:25

Edward Peters

3,623
2
16
39

score 1 · Answer 4 · answered Apr 07 '16 at 06:25

1

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.

answered Apr 07 '16 at 06:25

Ji aSH

3,206
1
10
18

2

You can catch AssertionError thrown by assert. – titogeo Apr 07 '16 at 06:27
You can use `IllegalStateException` or `IllegalArgumentException` for this (depending on whether `list` is an instance variable or a method argument, respectively) – Erwin Bolwidt Apr 07 '16 at 06:34
@titogeo but then your methods does not get a throws signature, which somehow makes the exception invisible and irrelevant :) – Ji aSH Apr 07 '16 at 06:38

Is "assert false;" a good practice?

4 Answers4

Linked