Java instantiate private variables

Question

Is it a bad practice to instantiate private variable in a class to avoid the NullPointerException when using the variable, for example:

public Class MyClass{

private HashMap<String, String> myHashMap = new HashMap<String, String>();


public HashMap<String, String>  getMyHashMap (){return myHashMap;  }
public HashMap<String, String>  setMyHashMap (String myString){ /* treatment to set the myHashMap under some conditions */}
}

If we don't instantiate the myHashMap, the method getMyHashMap() may return null Is it a better practice that the caller of getMyHashMap() checks for the null value? Is there a coding good practice to be applied in those circumstances?

Take a look at programmers.stackexchange.com/questions/120355/… — antonio
– antonio, Commented Nov 10, 2015 at 10:33

hsz · Accepted Answer · 2015-11-10 10:33:00Z

3

I use two solutions for this kind of situations. If myHashMap has to be always available, set it as final and initialize with value:

private final HashMap<String, String> myHashMap = new HashMap<String, String>();

public HashMap<String, String> getMyHashMap () {
    return myHashMap;
}

If myHashMap is something bigger than empty object I rather preffer to do something like a lazy loading:

private HashMap<String, String> myHashMap;

public HashMap<String, String> getMyHashMap () {
    if (myHashMap == null) {
        myHashMap = new HashMap<String, String>();
    }
    return myHashMap;
}

answered Nov 10, 2015 at 10:33

hsz

153k63 gold badges268 silver badges320 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Filippo Fratoni Over a year ago

In case of concurrent access it may result in two callers obtaining references to different maps. I think the second piece of code should be synchronized

biziclop Over a year ago

@FilippoFratoni If it is designed to use concurrently, yes. Or you can just document that your class isn't thread safe and requires external synchronization. (Which may be adequate in some cases, not in others.)

divesh premdeep · Accepted Answer · 2015-11-10 10:38:02Z

1

It is perfectly ok.

A common pattern in programming is iterating over a collection or map that is returned from a method. If the method possibly returns null, the onus is on the caller to perform a null check before attempting to iterate over the collection or map. Thus, it is normally considered bad practice to return null from public methods that return a collection or map. A better alternative would be to return an empty collection or map, as these allow safe iteration without a NullPointerException.

While not recommended, it is ok to return null from a private method that returns a collection or map, since their scope is limited to the enclosing class.

answered Nov 10, 2015 at 10:38

divesh premdeep

1,0703 gold badges16 silver badges30 bronze badges

Comments

Andy Turner · Accepted Answer · 2015-11-10 10:52:28Z

No. It is not a bad practice.

By doing this, you are establishing up an invariant for the class: myHashMap is not null. This invariant makes reasoning about the correct functioning of the rest of the class easier: you know that myHashMap is not null, so you won't get NullPointerException when you try to access it.

You can implement this by assigning the value in an initializer block (which your code is equivalent to) or assigning the value in the constructor. The two are roughly equivalent; actually, assigning the field when you declare it becomes an initializer block, which is executed before the constructor.

You can strengthen your reliance upon this invariant by making myHashMap final: this means that you can't accidentally set it to null later.

You don't strictly need this invariant: as @hsz suggests, you can make the value non-null lazily. However, I would argue that this clutters the class, and makes it harder to reason about. It's also harder to change the class to add new functionality: you have to remember to add that lazy instantiation to new code if it requires access to myHashMap.

I would say it is much better to create it once-and-for-all when you create the instance, and get on with the rest of your code - especially for something as trivial as an empty HashMap.

Héctor Valls · Accepted Answer · 2015-11-10 10:34:28Z

0

You could instantiate your map in the constructor.

public Class MyClass{

private HashMap<String, String> myHashMap;

public MyClass(){
    myHashMap = new HashMap<String, String>();
}


public HashMap<String, String>  getMyHashMap (){ return myHashMap;  }
public HashMap<String, String>  setMyHashMap (String myString){ /* treatment to set the myHashMap under some conditions */}
}

This way, getMyHashMap will never return null

answered Nov 10, 2015 at 10:34

Héctor Valls

26.4k43 gold badges153 silver badges267 bronze badges

1 Comment

biziclop Over a year ago

This is functionally equivalent. (So long as you don't create another constructor.)

Collectives™ on Stack Overflow

Java instantiate private variables

4 Answers 4

2 Comments

Comments

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

2 Comments

Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related