Is using Objects.requireNonNull() in production bad practice?

Question

Objects.requireNonNull is a helpful method to spot NullPointerExceptions effectively while developing. But in fact it's throwing a RuntimeException which could crash the application (the worst thing ever in production). Which brings up the question - is it safe to use this method on production or just in development?

Answer 1

Objects.requireNonNull is basically function precondition. This is very similar to assertions since they can also be used as preconditions. So asking whether we should use Objects.requireNonNull is the same as asking whether we should keep assertions in development and in production. Opinions of experts:

The Pragmatic Programmer:

Leave Assertions Turned On

...

Turning off assertions when you deliver a program to production is like crossing a high wire without a net because you once made it across in practice. There’s dramatic value, but it’s hard to get life insurance.

Code Complete:

Normally, you don’t want users to see assertion messages in production code; assertions are primarily for use during development and maintenance. Assertions are normally compiled into the code at development time and compiled out of the code for production. During development, assertions flush out contradictory assumptions, unexpected conditions, bad values passed to routines, and so on. During production, they can be compiled out of the code so that the assertions don’t degrade system performance.

...

For highly robust code, assert and then handle the error anyway

Other thoughts in ShipWithAssertionsOn

My opinion:

You should use it for situations:

1. "should never happen"
2. "there is no way to recover"

Let's play devil's advocate:

Suppose customer is making an order and you get an error in sendMail function that sends email about order state to customer. Since program broke in the middle of order it is not clear what is the state of that order. Order could be made without sending email to customer. Since making an order is primary objective we should not assert in sendMail.

Suppose that we did not add assert in sendEmail the program still crashed because of invalid arguments Fail-Fast tells us that we should have added assert. Since that would give us more knowledge about the error.

Suppose that we added error handling in sendEmail for invalid parameters. We notice the problem some time later from error logs and some customers complaining. Other customer still can make an order and don't notice missing email. Now we are left with dilemma of what other functions should have error handling too.

Answer 2

When faced with an error good programs fail early and obviously.

Consider the following program:

class EmailService {

  EmailClient client;

  EmailService(EmailClient client){
    this.client = client;   
  }

  sendHighlyImportantEmail(User user){
    this.client.send(user.getEmail(), "Highly important");
  }
} 

Setting up EmailService with new EmailService(null) would be an error because it would cause sendHighlyImportantEmail to fail.

Now this program can either fail when an important thing happened and sendHighlyImportantEmail is called, or by using this.client = requireNonNull(client) the program will fail to boot.

Now you have to evaluate which failure mode is worse:

A: A program that reliably fails to boot and doesn't work at all.
B: A program that fails in a very specific hard to reproduce situation.

On its face mode A sounds much worse. You have no functionality at all while with mode B you can still use 99% of the application. But this is a false sense of security. The program is equally broken in either situation. The error is just less visible.

It much more likely that a program with failure mode B will end up in production, where as a program with failure mode A is unlikely to make it past the first developer that tries to start the app.

So when faced with an error good programs fail early and obviously.