Value checking logic inside or outside of a class ?

Question

Take this skeleton class that someone wants to fill in to fetch RSS streams on a series of web sites:

public class RSSStream extends Thread {

    public RSSStream(String rssStreamName,String rssURL,int refreshTime){
     // constructor code goes here
    }
}

Now, let's consider that refreshTime has to be higher than zero and that rssURL should be a valid http address.

The obvious reflex is to have some value checking logic inside the constructor. However, a call to the constructor instantiates an Object whatever happens. This means the Object ends up being useless if the values don't allow it to do it's job. This also means that the Object should be eventually dumped or reused.

So, here's a couple of questions on the subject:

• Why do some classes impose a getInstance() method coupled with what is probably a private constructor ? If I remember well, one example would be GregorianCalendar.
• In what cases would you use this same approach ?
• In most cases do you have the checking logic in your constructor ?
• If so, do you apply this or not to Entity-style classes used in the persistence context of a domain model ?

All your answers are welcome. It'll be interesting to get a clear view of the most common practise is.

Answer 1

If the value checking you need to do may leave the object instance in a corrupt state then you should consider creating a factory to generate your instances. This will allow you to throw an exception in the event that the arguments that are passed are not valid.

If, however, your object can be "fixed" in the constructor with default values in the event that the validation fails then use default values to provide the caller with an object that is useless but not corrupt.

Answer 2

A few of the most common things:

• If you use FactoryMethod for the construction of complex objects you'll typically have a private constructor and require instantiation through the factory. This supports switching construction strategies with the factory.

• Clearest to me would be throw a custom ( and hopefully informative ) Exception in the constructor if instantiation can't happen: :

  public class Person {

      public Person(Integer id, String name) throws InvalidPersonException
           if (name==null) throw new InvalidPersonException("You cant have a person without a name");
           ...
  

• This is likely to create a mess in entity persistence objects, mostly because you want these objects to be logic-free, and because the framework should handle this for you - e.g. if you have a bean containing (id, name) in hibernate and try to persist into a table with the name required to be non-null, the error the db throws or from your configuration should be sufficient.

Answer 3

If your constructor throws an exception, the reference to your object is not returned so the object can be immediately garbage collected. So you are not wasting memory. I think this is the right place to do your validation whenever you are constructing objects, especially immutable objects.

getInstance() is a factory method that returns an instance of a subclass. You use this when you are returning a different subclass depending on circumstances. For example, Calendar.getInstance() returns a GregorianCalendar in my locale but it may return a different implementation if my locale settings are different.

For various patterns for creating objects, check out http://en.wikipedia.org/wiki/Creational_pattern

Hope this helps

Answer 4

• I've primarily seen and used a static getInstance() and private constructor just for use of the Singleton pattern. However, Java's Calendar abstract class uses this method not for a Singleton, but for instantiating a default implementation (Gregorian) with a default timezone and locale. Possibly this is done because Calendar is abstract and can't be instantiated. GregorianCalendar actually has public constructors.
• In most cases, I'd just a no-argument public constructor and setter methods, and place the checking logic and exception throwing in the setter methods. I frequently use Spring's dependency injection, and the @Required annotation makes it possible to have the container throw an exception if all the required attributes aren't set.
• I don't apply this sort of logic in entity objects, I try to keep all logic out of them for simplicity's sake, and make them strictly POJOs.

Answer 5

In this particular case I would just do the argument validation in the constructor and throw an IllegalArgumentException with the descriptive error message. The object would not be created and the calling code would have to be repaired.

You can do this in a getInstance() method too. One reason to use a getInstance() is if the instances are very sharable, to cut down on the number of objects you create. For example if you can have multiple RSSStream objects with the same URL, you could rig up a getInstance() to share those instances. (This assumes the instances are immutable.)

There's also the Builder pattern. You'd have a nested inner class called RSSStream.Builder and say things like:

RSSStream rss = new RSSStream.Builder.build(url)
                                     .name("stack overflow")
                                     .refreshTime(2)
                                     .build();

This pattern makes your client code more self-descriptive and also ensures you never build an RSSStream in an invalid state (the methods of the builder throw IllegalArgumentException). A builder is flexible enough to replace all of your public constructors.

I've generally used IllegalArgumentException, throwing it from the constructor, or a validation method (to factor the error checking code out of multiple constructors). I've started to use the Builder method for my more important public APIs and really like it.

You can certainly use these techniques in your domain model classes.

Answer 6

Constructors that need to validate their arguments typically throw an IllegalArgumentException if the validation fails. When a constructor throws an exception, no "halfway initialised" object is left to worry about.