Factory pattern: Validating input when creating a bean (Guice/Guava)

Question

I'm writing a new app, using Guice for dependency injection, and Guava's Preconditions for attribute validation.

I'm using the factory pattern to create beans based on external input. The question is, what would be the preferred way to validate the input? (In terms of maintainability, clarity, etc.)

Let's assume these classes:

Bean.java

public interface Bean {

    public Object getFoo();

}

BeanImpl.java

public class BeanImpl implements Bean {

    private final Object foo;

    public BeanImpl(Object param) {
        foo = param;
    }

    @Override
    public String getFoo() {
        return foo;
    }

}

I want to check that "param" holds a valid value for "foo":

Preconditions.checkArgument(SomeValidator.isValid(param), "Bad param: %s!", param);

Where should I do that and why?

• In BeanImpl's constructor? (I'm unsure why but adding checks in the constructor feels like a bad idea.)
• Before calling BeanFactory.create(param)? (Sounds like terrible code duplication.)
• Through some Guice mechanism I am unaware of?

Answer 1

If it is invalid to ever construct a BeanImpl with a particular foo value, then throw an IllegalArgumentException in the constructor detailing what is wrong.

public class BeanImpl {
  ...
  public BeanImpl(Object param) {
    if (param == null) {
      throw new IllegalArgumentException("Param cannot be null");
    }
    foo = param;
  }
  ...
}

From the early definitions of objects being "state and behavior combined" this enforces that you do not pollute your objects with invalid state.

As far as the factory, it is a convenience to check values before constructing objects, but the factory's true responsibilities do not extend into the object's responsiblities. The factory pattern is good for making sure that an Object corresponds to others in meaningful ways, but it shouldn't do the Object's job of making sure the Object's internal state is correct.

Having one Object (the factory) making sure another Object's state is correct is misplaced behavior. You put the behavior (verifying state) in the Factory, but the state is in the Object. As a result, you now have decentralized connected code that is supposed to be tightly coupled. It means that now you will have to have a Factory to make an Object, instead of having a Factory to make an object when it makes sense, and making the Object directly when it doesn't.

This often comes out in unit testing. If you didn't put the verification code in the factory, you could unit test the Object independently; testing for bad parameters. However, if you put the verification code in the Factory, you would not be able to construct "valid" Objects without the Factory, which is the first hint you will get that you have unnaturally coupled two objects that shouldn't be tightly coupled together.

Of course, there are exceptions; but they tend to come when one expects the data to be right, but the manner in which it is collected didn't provide validation. For example, as structured record collected from a network socket might actually have data that is not conceptually internally consistent; but, due the the nature of the processing, invalid input is handled, logged, and discarded.