views:

2715

answers:

9

Hey,

I've been wondering what the best (i.e. cleanest/safest/most efficient) way of handling multiple constructors in Java is? Especially when in one or more constructors not all fields are specified:

public class Book
{

    private String title;
    private String isbn;

    public Book()
    {
      //nothing specified!
    }

    public Book(String title)
    {
      //only title!
    }

    ...     

}

What should I do when fields are not specified? I've so far been using defualt values in the class so that a field is never null, but is that a "good" way of doing things?

+21  A: 

A slightly simplified answer:

public class Book
{
    private final String title;

    public Book(String title)
    {
      this.title = title;
    }

    public Book()
    {
      this("Default Title");
    }

    ...
}
Craig P. Motlin
+1 I find it a good rule of thumb that all your constructors should go through a common "choke point".
cletus
Also, it's always advisable to start every constructor with this() or super() =8-)
Yuval
Constructor chaining is always good.
R. Bemrose
+1 for constructor chaining - for extra marks, "title" can probably be declared final as it's unlikely to change during a Book object's lifetime (immutability is really good!).
Andrzej Doyle
You should probably also throw an exception if title is null in the constructor, that way you can guarantee that it will NEVER be null in a method (which can be extremely helpful). This is a good habit to do whenever possible (final + not-null).
Bill K
Be careful with the null checks. http://misko.hevery.com/2009/02/09/to-assert-or-not-to-assert/
Craig P. Motlin
The misko post is not one I would agree with. I'd rather suck up testing hardshot to ensure that production code is better.
TofuBeer
Nice but won't always be enough: you could have two optional arguments for a total of four constructors, i.e. A AB AC ABC. In this case you can't fall back everything to a lesser constructor: either ABC-->AB-->A (and you leave AC) or ABC-->AC-->A (and you leave AB).
Lo'oris
+11  A: 

Consider using the Builder pattern. It allows for you to set default values on your parameters and initialize in a clear and concise way. For example:


    Book b = new Book.Builder("Catcher in the Rye").Isbn("12345")
       .Weight("5 pounds").build();

Edit: It also removes the need for multiple constructors with different signatures and is way more readable.

kgrad
This is best idea if you have multiple constructor, you can easily expand for new types and less error prone
Legend
@Dinesh - I agree, I use Builders all over my code. I love the pattern!
kgrad
In my opioni it like a Fluent Interface (http://www.codemonkeyism.com/archives/2007/10/10/fluent-interface-and-reflection-for-object-building-in-java/)
alepuzio
However, the Builder can be a bit of a nuisance if you want to extend it (http://stackoverflow.com/questions/313729/how-to-implement-and-extend-joshuas-builder-pattern-in-net).
cdmckay
+5  A: 

You need to specify what are the class invariants, i.e. properties which will always be true for an instance of the class (for example, the title of a book will never be null, or the size of a dog will always be > 0).

These invariants should be established during construction, and be preserved along the lifetime of the object, which means that methods shall not break the invariants. The constructors can set these invariants either by having compulsory arguments, or by setting default values:

class Book {
    private String title; // not nullable
    private String isbn;  // nullable

    // Here we provide a default value, but we could not define the 
    // parameterless constructor, so that providing a title is required
    public Book()
    {
        this("Untitled"); 
    }

    public Book(String title) throws IllegalArgumentException
    {
        if (title == null) 
            throw new IllegalArgumentException("Book title can't be null");
        this.title = title;
        // leave isbn without value
    }
    // Constructor with title and isbn
}

However, the choice of these invariants highly depends on the class you're writing, how you'll use it, etc., so there's no definitive answer to your question.

Luc Touraille
You want to check that null isn't being passed to that second constructor.
Tom Hawtin - tackline
Indeed, I'll add that.
Luc Touraille
A: 

Your code is fine.

You should always do a null check on your fields regardless or your construction pattern.

You shouldn't have a default value on your object fields for the sake of having a default value. Only have a default value if you need one, otherwise null is your default.

rick schott
+2  A: 

Another consideration, if a field is required or has a limited range, perform the check in the constructor:

public Book(String title)
{
    if (title==null)
        throw new IllegalArgumentException("title can't be null");
    this.title = title;
}
Steve Kuo
You should be doing this for all your public methods. Always check the arguments, it'll save you some headaches down the road.
cdmckay
+1  A: 

Some general constructor tips:

  • Try to focus all initialization in a single constructor and call it from the other constructors
    • This works well if multiple constructors exist to simulate default parameters
  • Never call a non-final method from a constructor
    • Private methods are final by definition
    • Polymorphism can kill you here; you can end up calling a subclass implementation before the subclass has been initialized
    • If you need "helper" methods, be sure to make them private or final
  • Be explicit in your calls to super()
    • You would be surprised at how many Java programmers don't realize that super() is called even if you don't explicitly write it (assuming you don't have a call to this(...) )
  • Know the order of initialization rules for constructors. It's basically:

    1. this(...) if present (just move to another constructor)
    2. call super(...) [if not explicit, call super() implicitly]
    3. (construct superclass using these rules recursively)
    4. initialize fields via their declarations
    5. run body of current constructor
    6. return to previous constructors (if you had encountered this(...) calls)

The overall flow ends up being:

  • move all the way up the superclass hierarchy to Object
  • while not done
    • init fields
    • run constructor bodies
    • drop down to subclass

For a nice example of evil, try figuring out what the following will print, then run it

package com.javadude.sample;

/** THIS IS REALLY EVIL CODE! BEWARE!!! */
class A {
    private int x = 10;
    public A() {
        init();
    }
    protected void init() {
        x = 20;
    }
    public int getX() {
        return x;
    }
}

class B extends A {
    private int y = 42;
    protected void init() {
        y = getX();
    }
    public int getY() {
        return y;
    }
}

public class Test {
    public static void main(String[] args) {
        B b = new B();
        System.out.println("x=" + b.getX());
        System.out.println("y=" + b.getY());
    }
}

I'll add comments describing why the above works as it does... Some of it may be obvious; some is not...

Scott Stanchfield
1. A's init() method is never called when creating a B (B overrides it)
Scott Stanchfield
2. B's init() method is called from A's constructor
Scott Stanchfield
B's init() is called *before* B's initializers are called!. y gets assigned to 42 *after* it's assigned in init()
Scott Stanchfield
Never say never - there are places where calling protected methods from a constructor is exactly the right thing to do, in order to deliberately defer some specifics of construction into subclasses.
Software Monkey
Nope - I'll stick with "never".If you want to defer construction specifics to subclasses, that's what the subclass constructors are for.The trouble with calling non-finals from constructors is that the subclass has not yet been initialized - just plain unsafe!
Scott Stanchfield
A: 

I would do the following:

public class Book
{
    private final String title;
    private final String isbn;

    public Book(final String t, final String i)
    {
        if(t == null)
        {
            throw new IllegalArgumentException("t cannot be null");
        }

        if(i == null)
        {
            throw new IllegalArgumentException("i cannot be null");
        }

        title = t;
        isbn  = i;
    }
}

I am making the assumption here that:

1) the title will never change (hence title is final) 2) the isbn will never change (hence isbn is final) 3) that it is not valid to have a book without both a title and an isbn.

Consider a Student class:

public class Student
{
    private final StudentID id;
    private String firstName;
    private String lastName;

    public Student(final StudentID i,
                   final String    first,
                   final String    last)
    {
        if(i == null)
        {
            throw new IllegalArgumentException("i cannot be null"); 
        }

        if(first == null)
        {
            throw new IllegalArgumentException("first cannot be null"); 
        }

        if(last == null)
        {
            throw new IllegalArgumentException("last cannot be null"); 
        }

        id        = i;
        firstName = first;
        lastName  = last;
    }
}

There a Student must be created with an id, a first name, and a last name. The student ID can never change, but a persons last and first name can change (get married, changes name due to losing a bet, etc...).

When deciding what constrructors to have you really need to think about what makes sense to have. All to often people add set/get methods because they are taught to - but very often it is a bad idea.

Immutable classes are much better to have (that is classes with final variables) over mutable ones. This book: http://books.google.com/books?id=ZZOiqZQIbRMC&pg=PA97&sig=JgnunNhNb8MYDcx60Kq4IyHUC58#PPP1,M1 (Effective Java) has a good discussion on immutability. Look at items 12 and 13.

TofuBeer
Why do you make the parameters final?
Steve Kuo
final parameters make it so that you cnnot accidentaly do t = title for example.
TofuBeer
A: 

Several people have recommended adding a null check. Sometimes that's the right thing to do, but not always. Check out this excellent article showing why you'd skip it.

http://misko.hevery.com/2009/02/09/to-assert-or-not-to-assert/

Craig P. Motlin
A: 

You should always construct a valid and legitimate object; and if you can't using constructor parms, you should use a builder object to create one, only releasing the object from the builder when the object is complete.

On the question of constructor use: I always try to have one base constructor that all other defer to, chaining through with "omitted" parameters to the next logical constructor and ending at the base constructor. So:

class SomeClass
{
SomeClass() {
    this("DefaultA");
    }

SomeClass(String a) {
    this("DefaultA","DefaultB");
    }

SomeClass(String a, String b) {
    myA=a;
    myB=b;
    }
...
}

If this is not possible, then I try to have an private init() method that all constructors defer to.

And keep the number of constructors and parameters small - a max of 5 of each as a guideline.

Software Monkey