CODEWITHSUNDEEP
java
sabrina2020
sabrina2020

Reputation: 2462

Java instantiate private variables

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?

Upvotes: 2

Views: 1084

Answers (4)

Andy Turner
Andy Turner

Reputation: 140309

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.

Upvotes: 1

divesh premdeep
divesh premdeep

Reputation: 1070

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.

Upvotes: 1

H&#233;ctor
H&#233;ctor

Reputation: 26034

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

Upvotes: 0

hsz
hsz

Reputation: 152206

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;
}

Upvotes: 3

Related Questions