Recursion and iteration how can i optimize this code

Question

I have written some code which will fetch contents from a resource which is actually stored in a tree format. Since it is in a tree format, there will be a parent-child relation and hence recursion.

I am facing lot of performance issue as tree is growing in size this particular piece of code takes up-to 25 sec which is very bad. This piece of code will basically read data stored in file system (just for example) each content has certain set of property which it has to read.

import java.util.List;
public class Links {
    private String nodeName;
    private List<Links> children;

    public List<Links> getChildren() {
        return children;
    }

    public void setChildren( List<Links> children ) {
        this.children = children;
    }

    public String getNodeName(){
        return nodeName;
    }

    public void setNodeName( String nodeName ){
        this.nodeName = nodeName;
    }
}

package menu;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;

public class Utility {
    /*
     * IN this class NavModel,CModel,CNode are some dummy classes which help us read contents form some resources which
     * are stored in dummy format as Example of tree format stored data below is the example tree child1 child2 child3
     * child3-1 child:q child:r child:a child3-2 child4
     */

    private static void populateLinks( NavModel navModel,
            ContentModel metaDataModel, Object objectNode, Links parent,
            boolean specialLinks ) throws IOException{
        try{
            List<Links> childLinks = new ArrayList<Links>();
                    // below code gets all the childrens of parent
            Iterator it = navModel.getChildren( objectNode );
            while( it.hasNext() ){
                NavNode node = (NavNode) it.next();
                ContentNode contentNode = node.getContentNode();

                Links links = new Links();

                MetaData data = metaDataModel.getMetaData( contentNode );

                Map<String, String> paramValues = new HashMap<String, String>();
                             // this particular piece of 
                             // code gets all the properties of content iterates and stores it in data structure
                Iterator metaDataIterator = data.getNames().iterator();
                while( metaDataIterator.hasNext() ){
                    String pagePropertyKey = metaDataIterator.next().toString();
                    String pagePropertyValue = (String) data
                    .getValue( pagePropertyKey );

                    if( pagePropertyKey.equalsIgnoreCase( "LINK_TYPE" ) ){
                        links.setLinkType( pagePropertyValue );
                    }
                    else if( pagePropertyKey.equalsIgnoreCase( "TCDID" ) ){
                        links.setTcmIdMap( pagePropertyValue );
                    }
                    else{
                        paramValues.put( pagePropertyKey, pagePropertyValue );
                    }
                }

                links.setParamValues( paramValues );

                if( specialLinks ){
                    links.setNodeName( links.getParamValues().get(
                    "APPLICATIONNAME" ) );
                }
                else{
                    links.setNodeName( contentNode.getTitle( new Locale(
                    "en_US" ) ) );
                }

                links.setDisplayName( setAppDispName( links.getNodeName() ) );
                childLinks.add( links );

                if( navModel.hasChildren( node ) ){
                                    // This is where recursion happens
                    populateLinks( navModel, metaDataModel, node, links,
                            specialLinks );
                }
            }
            parent.setChildren( childLinks );

        }
        catch( Exception e ){
        }
    }

  // THis is the method which calls the recursion function
   public static Links setupLinks( String categoryLinkName, String name ){
    Links categoryLinks = null;
    CModel contentModel = new CModel();
    NavModel navModel = new NavModel();
    categoryLinks = Utility.createCategoryLinks( categoryLinkName );
    Object objectNode = contentModel.getLocator().findByUniqueName( name );
    if( objectNode != null ){
        if( navModel.hasChildren( objectNode ) ){
            populateLinks( navModel, objectNode, categoryLinks );
        }
    }
}

    private static Object setAppDispName( String nodeName ){
        // fetch value from db
        return null;
    }    
}

I know code is not complete and may be in appropriate to review but I can't paste the whole of my code hence I hope you understand and review there is recursion and iteration any way I can better write this piece of code.

Answer 1

Iterator it = navModel.getChildren( objectNode );
while( it.hasNext() ){
        NavNode node = (NavNode) it.next();

java.util.Iterator has an extending class, which if you use, you don't have to manally cast back after calling .next(), as follows:

Iterator<NavNode> it = navModel.getChildren( objectNode );
while( it.hasNext() ){
    NavNode node = it.next();

Furthermore, if you are using Java 7 or above, you no longer have to specify the extending type in the assignment if you use the "diamond operand", as in these lines:

Map<String, String> paramValues = new HashMap<>();

List<Links> childLinks = new ArrayList<>();

Additionally, you are calling .equalsIgnoreCase() (See java.lang.String source code) multiple times in an if-else statement, which internally calls .toUpperCase() on the String, which can happen twice in a worst case scenario of your code, therefore I believe this would be more efficient:

pagePropertyKey = pagePropertyKey.toUpperCase();
switch(pagePropertyKey)
{
    case "LINK_TYPE": links.setLinkType( pagePropertyValue );
                      break;
    case "TCDID":     links.setTcmIdMap( pagePropertyValue );
                      break;
    default:          paramValues.put( pagePropertyKey, pagePropertyValue );
                      break;
}

As then it only has to call .toUpperCase() once and then equals() alone which is more efficient.