Search code examples
javaoopobjectsoa

Null checks in constructor vs in service method?


This is a topic in one of our code reviews, I'd like more opinions.

Let's say I am writing a service that will allow me to insert a simple Person object into a database.

public class Person
{
   private String name;
   ....
}

We have a simple VerifyNotNull method throws IllegalArgumentException.

Which route would you take and why.

Option 1:

Verify not null in constructor of Person object.

public Person(String name)
{
     VerifyNotNull(name);//throws illegal arg exception if name is null
     this.name = name;
}


Option 2:

Allow Person to be constructed with null, verify not null on addPerson call.

public class PersonService
{
  public void addPerson(Person personToAdd)
  {
     VerifyNotNull(personToAdd.getName());//throws illegal arg exception
     //add code
  }
}

I don't like the idea of throwing Exceptions in constructors. To me option 2 feels right, I don't know how to explain or justify it though.

Is it acceptable to throws Exceptions in constructors?

Thanks for your help!


Solution

  • The first approach is more fail-fast, which will increase the likelihood that you'll find the source of your bug more quickly. Think of it like this: if your error log starts telling you that a number of errors have been cropping up because you're trying to add people that have null names, you're going to want to know where those people's null names came from, right? Depending on how your system is structured, it's possible that the person was created miles away from the place where the person is getting added to the service. So you have no idea which of the four thousand places in your code is creating people without names.

    So if I had to choose, I'd go with the first option. If throwing exceptions in the constructor feels like an anti-pattern, make the constructor private and make people construct your object through a static factory method. It's perfectly reasonable to expect a factory method to throw exceptions on bad input.

    Of course, it depends on your business model. If a person will a null name is a perfectly legal thing to create when you're in the data-entry phase, and it's not until you're getting ready to persist that person's information that you want to make sure it's passed validation, then that's a different story. In that case, you might even want to come up with a ValidatedPerson class that wraps Person, but indicates in a type-safe way that the addPerson method can only be called if someone has gone through the validation process, because the only way to create a ValidationPerson is through a specific validate method that checks the person's name.