9

In the below class, I have a Map which I intend to use later in some function. Is it good practice that I intialize the Map with new Map(); or since it will get a value later when the method is invoked would it be good to initialize it later. Does it really matter? What is a good practice and why?

public class Hello{
    private static Map<String,String> aMap;
    // Lets assume I'm just using the default constructor for this class, 
    //    and there is no static initialization code.
    // All methods are static so will be access with classname.method()
    public static void fillTheMapWithCars(){
        aMap = Car.getSomeCars();
        doSomeStuff();
    }

    private static void doSomeStuff(){
        // does some stuff based on values populated in aMap
        // it means it will access keys and value without checking if
        // aMap is null. Since I had already initialized it in fillTheMapWithCars  
        // there wont be any runtime errors accessing aMap. I hope! 
        // This is private method so other class can't manipulate aMap inside here
    }
}

Car

public class Car{
    public static Map<String,String> getSomeCars{
        Map<String,String> carMap = new Map<String,String>();
        // does some stuff and returns a Map
        return carMap;
    }
}

3 Answers 3

13

The basic rule of thumb is to initialize a resource when you need it, and no later. Ideally, you should only need to check for initialization infrequently (preferably no more than once, or never). It's often possible to write code where no null checks are ever made, because none are needed.

In this case, since aMap is never used before this assignment from the static method, there's no point in initializing it before it is initialized. To do so wastes CPU time, which is a precious resource.

For example, I often see inexperienced developers write this type of code:

List<Account> accts = new List<Account>();
accts = [SELECT Name FROM Account LIMIT 10];

This is a waste of CPU time, because memory was allocated for no reason, only to be immediately used again. While this is a minimal waste of resources, doing this often in various places will eat away at performance.

The opposite pattern is when you don't know if something is initialized and so you end up checking it over and over again:

void doSomething() {
  if(aMap == null) {
    aMap = Car.getSomeCars();
  }
  // continue on...
}

With a few rare exceptions, which you'll know when you get to them, you should avoid doing so. Usually, if you "need" this pattern, it actually means you should be initializing your structure in a central location, typically the constructor.

The only time this sort of pattern should be considered at all is when you're using lazy-loading techniques. However, even in that case, you should avoid calling it repeatedly, again because of the cost to CPU time.

As an example, consider this:

Map<Id, String> myKeyMap { get {
  if(myKeyMap == null) {
    myKeyMap = new Map<Id, String>();
  }
  return myKeyMap;
} }

If you access myKeyMap repeatedly, you are incurring the cost of a null check. Do this in a loop of 1,000 accesses, and you've probably added at least 1 second of CPU time to your execution time.

Instead, you should use myKeyMap only once by "caching" it:

Map<Id, String> localKeyMap = myKeyMap;

This results in improved performance while still providing lazy-loading on the resource.

From a performance perspective, there are two goals you should work towards:

  1. Do not initialize a variable more than once.
  2. Do not check if a variable is null.

However, these are only guidelines, and you'll find times you need to violate these guidelines. When you do need to, I suggest you leave a comment explaining why it was necessary. Once you get in the habit of following the guidelines, situations where it doesn't follow the guideline will stand out.

4
  • Great Insights! Very helpful.
    – Arnold
    Dec 15, 2017 at 2:16
  • 3
    Remember though - while it is fine not to always have a null check for a variable - when accessing an index in a list especially when querying a SOQL List - you don't want to always assume there is 1 record minimum unless you can guarantee it is the case such as being the child of a master detail relationship where the child cannot exist otherwise. but if you are unsure - you should at least be sure to check your list size before accessing a specific 0 index of the list.
    – Ronnie
    Dec 15, 2017 at 4:26
  • typo - 3rd example last line sb return myKeyMap;
    – cropredy
    Dec 15, 2017 at 6:20
  • 1
    @Ronnie List size has nothing to do with null checks. It's a similar concept in that you should know when you can guarantee certain edge case behaviors, but it's fairly peripheral to what is written in this answer.
    – Adrian Larson
    Dec 15, 2017 at 19:19
3

I think this largely comes down to the architecture of the application, and how many other devs will be using this code. I'd argue that while initialising it with an empty map may mean you don't have to check if aMap itself is null, any methods that depend on data in the map will have to check values returned using get() anyway, so any benefits are rather small.

Often in these situations the rule of thumb is to not surprise other devs down the road, but future proofing is hard, and getting overly complicated is easy when you're trying to be too flexible.

I'd stick with what you've got, and then if you see a good reason to change it, do so.

3
  • 3
    One caveat, make it final, otherwise you don't actually know it's non-null.
    – Adrian Larson
    Dec 15, 2017 at 0:34
  • @AdrianLarson If I make it - final private static Map<String,String> aMap = new Map<String,String>(); is it right that I won't be able to edit it later in a method?
    – Arnold
    Dec 15, 2017 at 2:18
  • 3
    @Arnold You will no longer be able to assign to that variable, which makes it impossible to nullify. You can, however, still write to the collection using put, clear, etc.
    – Adrian Larson
    Dec 15, 2017 at 2:19
2

"Is it good practice to initialize variable?" - Keep in mind that you are getting language-specific advice. In C, you get undefined values if you happen to use a variable before it is initialized, so in that situation always initializing variables on declaration is a good thing. In other languages such as Java, C#, or Go; if a zero value is a valid outcome, then leaving it at the default until mutated (called a "zeroValue" in Go) is reasonable.

The basic idea that applies to all languages is to minimize the scope of existence of variables as much as possible. Thinking like a functional programmer and eliminating as much state as possible: variables defined at the top of a 50 line function, but only used across two adjacent lines will have extra state existing for 48 lines more than it needs to. Think of a metric where you are penalized per line for each potentially readable/writable state. And you should be penalized for re-assignments as well. Most variables should only be assigned at declaration.

This often means just putting up some curly braces after a comment just to define a scope. Note that doing this not only declares a variable as late as possible, but removes it from scope as long as possible. When you truly minimize the scope of variables, you will need to consider whether two lines commute with each other as you sort the lines to minimize the scope.

0

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .