Should I keep instance variables in Java always initialized or not?

Question

I recently started a new project and I'm trying to keep my instance variables always initialized to some value, so none of them is at any time null. Small example below:

public class ItemManager {

  ItemMaster itemMaster;
  List<ItemComponentManager> components;

  ItemManager() {
    itemMaster = new ItemMaster();
    components = new ArrayList<ItemComponentManager>();
  }

  ...
}

The point is mainly to avoid the tedious checking for null before using an instance variable somewhere in the code. So far, it's working good and you mostly don't need the null-value as you can check also for empty string or empty list, etc. I'm not using this approach for method scoped variables as their scope is very limited and so doesn't affect other parts of the code.

This all is kind of experimental, so I'd like to know if this approach could work or if there are some pitfalls which I'm not seeing yet. Is it generally a good idea to keep instance variables initialized?

Answer 1

What happens if in one of your methods you set

itemMaster = null;

or you return a reference to the ItemManager to some other class and it sets itemMaster as null. (You can guard against this easily return a clone of your ItemManager etc)

I would keep the checks as this is possible.

Answer 2

Yes, it is very good idea to initialize all class variables in the constructor.

Answer 3

If it's all your code and you want to set that convention, it should be a nice thing to have. I agree with Paul's comment, though, that nothing prevents some errant code from accidentally setting one of your class variables to null. As a general rule, I always check for null. Yeah, it's a PITA, but defensive coding can be a good thing.

Answer 4

I would say that is totally fine - just as long as you remember that you have "empty" placeholder values there and not real data.

Keeping them null has the advantage of forcing you to deal with them - otherwise the program crashes. If you create empty objects, but forget them you get undefined results.

And just to comment on the defencive coding - If you are the one creating the objects and are never setting them null, there is no need to check for null every time. If for some reason you get null value, then you know something has gone catastrophically wrong and the program should crash anyway.

Answer 5

I usually treat an empty collection and a null collection as two separate things:

An empty collection implies that I know there are zero items available. A null collection will tell me that I don't know the state of the collection, which is a different thing.

So I really do not think it's an either/or. And I would declare the variable final if I initialize them in the constructor. If you declare it final it becomes very clear to the reader that this collection cannot be null.

Answer 6

The point is mainly to avoid the tedious checking for null before using a class variable somewhere in the code.

You still have to check for null. Third party libraries and even the Java API will sometimes return null.

Also, instantiating an object that may never be used is wasteful, but that would depend on the design of your class.

Answer 7

I have come across some cases where this causes problems. During deserialization, some frameworks will not call the constructor, I don't know how or why they choose to do this but it happens. This can result in your values being null. I have also come across the case where the constructor is called but for some reason member variables are not initialized.

In actual fact I'd use the following code instead of yours:

public class ItemManager {

  ItemMaster itemMaster = new ItemMaster();
  List<ItemComponentManager> components = new ArrayList<ItemComponentManager>();

  ItemManager() {
     ...
  }
  ...
}

Answer 8

An object should be 100% ready for use after it's constructed. Users should not have to be checking for nulls. Defensive programming is the way to go - keep the checks.

In the interest of DRY, you can put the checks in the setters and simply have the constructor call them. That way you don't code the checks twice.

Answer 9

First and foremost, all non-final instance variables must be declared private if you want to retain control!

Consider lazy instantiation as well -- this also avoids "bad state" but only initializes upon use:

class Foo {
    private List<X> stuff;
    public void add(X x) {
        if (stuff == null)
            stuff = new ArrayList<X>();
        stuff.add(x);
    }
    public List<X> getStuff() {
        if (stuff == null)
            return Collections.emptyList();
        return Collections.unmodifiableList(stuff);
    }
}

(Note the use of Collections.unmodifiableList -- unless you really want a caller to be able to add/remove from your list, you should make it immutable)

Think about how many instances of the object in question will be created. If there are many, and you always create the lists (and might end up with many empty lists), you could be creating many more objects than you need.

Other than that, it's really a matter of taste and if you can have meaningful values when you construct.

If you're working with a DI/IOC, you want the framework to do the work for you (though you could do it through constructor injection; I prefer setters) -- Scott

Answer 10

The way I deal with any variable I declare is to decide if it will change over the lifetime of the object (or class if it is static). If the answer is "no" then I make it final.

Making it final forces you to give it a value when the object is created... personally I would do the following unless I knew that I would be changing what the point at:

  private final ItemMaster itemMaster;
  private final List components;

  // instance initialization block - happens at construction time
  {
      itemMaster = new ItemMaster();
      components = new ArrayList();
  }

The way your code is right now you must check for null all the time because you didn't mark the variables as private (which means that any class in the same package can change the values to null).

Answer 11

From the name of the class "ItemManager", ItemManager sounds like a singleton in some app. If so you should investigate and really, really, know Dependency Injection. Use something like Spring ( http://www.springsource.org/ ) to create and inject the list of ItemComponentManagers into ItemManager.

Without DI, Initialization by hand in serious apps is a nightmare to debug and connecting up various "manager" classes to make narrow tests is hell.

Use DI always (even when constructing tests). For data objects, create a get() method that creates the list if it doesn't exist. However if the object is complex, almost certainly will find your life better using the Factory or Builder pattern and have the F/B set the member variables as needed.