System NullPointerException on get from map

Question

I have a nullpointer exception in the following code on line if(mapSoldAppStatus.containsKey(user.Id)){:

 Map<String,Map<String,Integer>> mapSoldAppStatus = ReportPageController.prepareMapObjectUnitPerStatus(Ids, status, 'userIddate__c');
for(userIddate__c user : userList){
    if(mapSoldAppStatus.containsKey(user.Id)){

    }
}

Do I only need to check if the map mapSoldAppStatus is empty before entering the loop or do I need to also check if the mapSoldAppStatus.containsKey(user.Id) is empty. I suppose if the user is not in the list then the .containsKey(user.Id) wont be looking for it.

Just want a confirmation if I am understanding this correctly.

Answer 1

Check whether the map is instantiated or not and if it is instantiated, you can use the containsKey method.

Map<String,Map<String,Integer>> mapSoldAppStatus = reportPageController.prepareMapObjectUnitPerStatus(Ids, status, 'userIddate__c');
for(userIddate__c user : userList){
    if(mapSoldAppStatus != null && mapSoldAppStatus.containsKey(user.Id)){

    }
}

Answer 2

You may want to use the Safe Navigation Operator to avoid an extra null check. Also, see my talk on using containsKey and get efficiently. It's better to:

Map<String,Map<String,Integer>> mapSoldAppStatus = ReportPageController.prepareMapObjectUnitPerStatus(Ids, status, 'userIddate__c');
for(userIddate__c user : userList){
  Map<String, Integer> userValues = mapSoldAppStatus?.get(user.Id);
  if(userValues != null) {
    // Do something with userValues
  }
}

In fact, I argue that containsKey should virtually never be used, as you almost always will be calling get later, so it makes sense to save the cost of the containsKey function and just check for null.

Further, I would argue that ReportPageController.prepareMapObjectUnitPerStatus is poorly written. You should try to write functions that never return a null value. It's an odd design that only adds overhead to every function you write. See articles like Say No to Null.