0
\$\begingroup\$

I've finally got all my code working as expected and it handles the data quickly. But now I'm looking to refactor it because there is some duplicate sections that just looks like can be broken down for more efficiency and readability. With out making this too long, here is the original:

I created functions createNumLabel and createPctLabel to basically do what is in the if statement. So if codeID = 1,32,28,33,10 call createNumLabel else call createPctLabel. While that doesn't crash the map no labels on the map are drawn. Any ideas would be helpful.

    function labels(graphics) {
    globals.map.on("update-end", function () {

        var font = new esri.symbol.Font(10, esri.symbol.Font.STYLE_NORMAL, esri.symbol.Font.VARIANT_NORMAL, esri.symbol.Font.WEIGHT_BOLDER, "Arial");//create font object
        //var gl = globals.featureLayers[1].graphics;//low res
        var gl2 = globals.featureLayers[2].graphics;//high res            

        console.log("Initial zoom level is :" + globals.map.getZoom());
        console.log("Scale is :" + globals.map.getScale());

        globals.map.graphics.clear();
        if(globals.map.getZoom() >=9)  {
            console.log(gl2.length);
            //globals.featureLayers[1].setVisibility(false);
            for (var i = 0; i < gl2.length; i++) {                   

                if (codeID == 1 || codeID == 32 || codeID == 28 || codeID == 33 || codeID == 10) {
                    //createNumLabel(graphics);
                    var g2 = globals.featureLayers[2].graphics[i];
                    var strLabel2 = g2.attributes.NAME + ":" + $.formatNumber(findFips(g2), { format: '#,###', locale: "us" });//creates string label formatted
                    var textSymbol2 = new esri.symbol.TextSymbol(strLabel2, font);

                    textSymbol2.setColor(new dojo.Color([0, 0, 0]));

                    var pt2 = g2.geometry.getExtent().getCenter();
                    var labelPointGraphic2 = new esri.Graphic(pt2, textSymbol2);
                    //add label to the map
                    globals.map.graphics.add(labelPointGraphic2);
                }//end if
                else {
                    //createPctLabel(graphics);
                    var g2 = globals.featureLayers[2].graphics[i];
                    var strLabelPct2 = g2.attributes.NAME + " : " + $.formatNumber(findFips(g2), { format: '#,###.0', locale: "us" }) + "%";
                    var textSymbol2 = new esri.symbol.TextSymbol(strLabelPct2, font);//create symbol with attribute name  

                    textSymbol2.setColor(new dojo.Color([0, 0, 0]));//set the color

                    var pt2 = g2.geometry.getExtent().getCenter(); //get center of county
                    var labelPointGraphic2 = new esri.Graphic(pt2, textSymbol2); //create label graphic

                    //add label to the map
                    globals.map.graphics.add(labelPointGraphic2);

                }//end else
            }//end for
        }//ends if            
    });//ends update end
}//ends labels function
\$\endgroup\$
1
\$\begingroup\$

Addressing the duplication, it seems that both conditions are processed the same with the exception of format. You could either provide an inline conditional (ternary statement) to determine the format to use or pass the codeID to a function and return the respective format. At that point the if/else narrows in context and is only applied at the building of strLabel/strLabelPct2.

The function usage would be more extensible if the need for code modifications were to arise, as such, it would be my preference.

Function:

var applyFormatting = function(codeID, data) {
  if (codeID == 1 || codeID == 32 || codeID == 28 || codeID == 33 || codeID == 10) {
    return  $.formatNumber(data, { format: '#,###', locale: "us" }); 
  }
  return $.formatNumber(data, { format: '#,###.0', locale: "us" }) + "%";
  };

for (var i = 0; i < gl2.length; i++) {                   
  var g2 = globals.featureLayers[2].graphics[i];

  var affectedValue = applyFormatting(codeID, findFips(g2));

  var strLabel2 = g2.attributes.NAME + ":" + affectedValue;
  var textSymbol2 = new esri.symbol.TextSymbol(strLabel2, font);
  textSymbol2.setColor(new dojo.Color([0, 0, 0]));
  var pt2 = g2.geometry.getExtent().getCenter();
  var labelPointGraphic2 = new esri.Graphic(pt2, textSymbol2);

  //add label to the map
  globals.map.graphics.add(labelPointGraphic2);
}  

Ternary:

 var codesThatAffectLabel = [1, 32, 28, 33, 10];

 for (var i = 0; i < gl2.length; i++) {                   
  var g2 = globals.featureLayers[2].graphics[i];
  var data = findFips(g2);

  var affectedValue = (-1 != codesThatAffectLabel.indexOf(codeID))
                    ? $.formatNumber(data, { format: '#,###', locale: "us" })
                    : $.formatNumber(data, { format: '#,###.0', locale: "us" }) + "%";

  var strLabel2 = g2.attributes.NAME + ":" + affectedValue;
  var textSymbol2 = new esri.symbol.TextSymbol(strLabel2, font);
  textSymbol2.setColor(new dojo.Color([0, 0, 0]));
  var pt2 = g2.geometry.getExtent().getCenter();
  var labelPointGraphic2 = new esri.Graphic(pt2, textSymbol2);

  //add label to the map
  globals.map.graphics.add(labelPointGraphic2);
}  
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.