How to improve the code to get a child node ValueMap

[anasustic]

I was getting a NullPointerException  in my method in the following line of code

ValueMap heroTeaserImageValueMap = resource.getChild("image").getValueMap()

so I rewrote it to:

ValueMap heroTeaserImageValueMap = getImageValueMap(heroTeaser);

private ValueMap getImageValueMap(Resource resource) {
    if (resource.hasChildren()) {
        return resource.getChild("image").getValueMap();
    } else {
        return ValueMap.EMPTY;
    }
}

This is not giving me the NullPointerException since I make sure I check if node has children nodes and from the data structure in the repo I know the child node name is image. Can someone suggest how I can improve this code. 

This is how the resource looks like in the repo

 "heroteaser": {
            "jcr:primaryType": "nt:unstructured",
            "targetBlank": "false",
            "bgColor": "blue",
            "teaserLead": "Home Hero Teaser Lead",
            "linkType": "button",
            "sling:resourceType": "xxx/heroTeaser",
            "teaserTitle": "Home Hero Teaser Title",
            "image": {
              "jcr:primaryType": "nt:unstructured",
              "fileReference": "/content/dam/xxx/Alltag_23-600x750.jpeg"
              }
            },

[Shailesh_Bassi_]

@anasustic you can re-write the code in the below format:

Resource heroTeaserImageResource = resource.getChild("image");
if(null == heroTeaserImageResource ){
    return;
}
ValueMap heroTeaserImageValueMap = heroTeaserImageResource.getValueMap();
if( null == heroTeaserImageValueMap ) {
   return;
}

This way there will be no case that you end up in "NullPointerException" and if required you can add up the logger statements if required.

Thanks

[anasustic]

Hi @Shailesh_Bassi_ 

I rewrote it like this implementing your suggestion:

private ValueMap getImageValueMap(Resource resource) {
    ValueMap heroTeaserImageValueMap = ValueMap.EMPTY;
    if (resource.hasChildren()) {
        Resource heroTeaserImageResource = resource.getChild("image");
        if (heroTeaserImageResource == null) {
            return heroTeaserImageValueMap;
        }
        heroTeaserImageValueMap = heroTeaserImageResource.getValueMap();
        if (heroTeaserImageValueMap == null) {
            return heroTeaserImageValueMap;
        }
    }
    return heroTeaserImageValueMap;
}

[Shailesh_Bassi_]

@anasustic There is no need for this explicit check "resource.hasChildren()" as, when you say  "resource.getChild("image"), either it will return you the resource or a null.
So this is an extra overhead over here in the code.

Thanks

[anasustic · Accepted solution]

Thank you @Shailesh_Bassi_ 

I simplified the code like this:

private ValueMap getImageValueMap(Resource resource) {

    Resource heroTeaserImageResource = resource.getChild("image");
    if (heroTeaserImageResource == null) {
        return ValueMap.EMPTY;
    }
    return heroTeaserImageResource.getValueMap();
}