Highlight countries based on category

Question

In the below code I should highlight countries on the map based on the category of the country in which there are 2 different type of countries (issuing office and selected countries). Based on the country type I have to highlight that country on the map. Let us not worry about map code.

There is a checkbox which contains names of all the countries once user choose checkbox against countries (i.e selects a country I should add it as issuing office and if user unchecks the checkgox then I should deselect the issuing office).

There are update details button onClicking. With this button I get some information which I will not show on the map (we can ignore action taken on clicking update details button for now).

Now after clicking update details button, if user selects a country by clicking checkbox, then I should add that country as selected country. If user unchecks the newly selected country I should deselect it from the map. If user unchecks the issuing office then we should deselect that country.

The idea here is first time user choose io after clicking update details buttons I get some information now which ever chooses newly they are selected countries which I highlight in different color after clicking on update details button I will convert all newly selected countries into issuing office.

Now that I have explained the functionality below few point:

Whenever a country I chosen or deselected I don't get the name of country that was choose/unchoosen. I get names of all the countries that currently selected.

Let's say I choose:

  Australia 

  my method gets 
  Australia

  now user chooses Italy 
  I get 

  Australia,Italy

    now user chooses Russia 
   I get 
     Australia,Italy,Russia

     Now user unchecks Australia
     I get 

     Italy,Russia

Below is the code. Please validate:

var issuingOffice,selectedCountries,issuingOfficeSymbol,mapDefaultSymbol,selectedOfficeSymbol;

require(["dojox/collections/ArrayList","esri/symbols/SimpleFillSymbol", "esri/symbols/SimpleLineSymbol", "dojo/_base/Color", "esri/graphic"
      ], function(arrayList){
            issuingOffice = new arrayList();
            selectedCountries = new arrayList();
            issuingOfficeSymbol = new SimpleFillSymbol(SimpleFillSymbol.STYLE_SOLID, new SimpleLineSymbol(SimpleLineSymbol.STYLE_SOLID, new Color("#AED8EC"), 1), new Color("#FDB913"));
            mapDefaultSymbol = new SimpleFillSymbol(SimpleFillSymbol.STYLE_SOLID, new SimpleLineSymbol(SimpleLineSymbol.STYLE_SOLID, new Color("#C2C2C2"), 1), new Color("#A0A0A0"));
            selectedOfficeSymbol = new SimpleFillSymbol(SimpleFillSymbol.STYLE_SOLID, new SimpleLineSymbol(SimpleLineSymbol.STYLE_SOLID, new Color("#C2C2C2"), 1), new Color("#8440E3"));
      });

function setIssuingOffice(countryName){
   console.log("function : setIssuingOffice");       

        /*
         *  semicolon sepearated list of countryName will be avaialabe via program selecton module
         *  we have to replace ; with , so that we iterate over the list
         */
        countryName = replaceAll(";", ",", countryName);     

        /*
         *  once we replace ; with , then we split the string with , to get
         *  list of string
         */
        var countryList = countryName.split(",");



        if(getDetailsClicked == true){


            var tempCountry;
            var removedCoutries;
            var countriesTobeRemoved;
            require(["dojox/collections/ArrayList"], function(arrayList){
                tempCountry = new arrayList();
                removedCoutries = new arrayList();
                countriesTobeRemoved = new arrayList();
            });


            // below code is to check if any country has been deselected
            var it = selectedCountries.getIterator();
            while (!it.atEnd()) {              
                var country = it.get();               
                var isSelected = false;          
                dojoArray.some(countryList, function(selectedCountry){
                        return isSelected=(country == selectedCountry);
                })                          
                if (!isSelected) {
                    tempCountry.add(country); // this is list of countris that has to be deleted on click on GEt Details
                }
            }



            // iterate over deselected countries and remove them
            // DO this when GET DETAILS is clicked.
            var tempCountryIt = tempCountry.getIterator();
            while (!tempCountryIt.atEnd()) {
                var country = tempCountryIt.get();            
                bottomBar.remove(country);
                selectedCountries.remove(country);
                if (country != producingOffice) {
                    removeCountry(country);
                }
            }

            var issuingIt = issuingOffice.getIterator();
            while (!issuingIt.atEnd()) {
                var country = issuingIt.get();
                var isSelected = false;
                dojoArray.some(countryList, function(selectedCountry){
                    return isSelected = (country == selectedCountry);
                })
                if (!isSelected) {
                    countriesTobeRemoved.add(country);
                }                            
            }         

            var countriesTobeRemovedIt = countriesTobeRemoved.getIterator();
            while (!countriesTobeRemovedIt.atEnd()) {
                var country = countriesTobeRemovedIt.get();               
                issuingOffice.remove(country);
                bottomBar.remove(country);
                if (country != producingOffice) {
                    removeCountry(country);
                }
            }


            dojo.forEach(countryList, function(country){          
                if (country != '' && country != undefined) {
                    if (!tempCountry.contains(country) && !selectedCountries.contains(country) && !countriesTobeRemoved.contains(country) && !issuingOffice.contains(country)) {                      
                        if (producingOffice != country) {
                            selectedCountries.add(country);
                            // TODO: check if country exists only then add 
                            if (!bottomBar.contains(country)) {
                                bottomBar.add(country);
                            }

                            // below is logic to highligt based on page if lannding page and if country has alert then highlight as alert.
                            if (hashMap.containsKey(country)) {
                                var graphic = hashMap.item(country);
                                graphic.setSymbol(selectedOfficeSymbol);
                                //map.graphics.add(graphic);
                            }
                        }else if(producingOffice == country){                       
                            selectedCountries.add(country);
                        }

                    }
                }
            });
        }else{
                var tempCountry;
                var removedCoutries;
                require(["dojox/collections/ArrayList"], function(arrayList){
                    tempCountry = new arrayList();
                    removedCoutries = new arrayList();
                });


                    // check for con-current modification ?
                    var isIt = issuingOffice.getIterator();
                    while (!isIt.atEnd()) {                       
                        var country = isIt.get();                        
                        var isSelected = false;
                        /*dojo.forEach(countryList, function(selectedCountry){
                            if (country == selectedCountry) {
                                isSelected = true;
                            }
                        });*/
                        dojoArray.some(countryList, function(selectedCountry){
                           return isSelected=(country == selectedCountry);
                        })                       
                        if (!isSelected) {
                            removedCoutries.add(country);
                        }
                    }

                    var tempCountryIt = removedCoutries.getIterator();
                    while (!tempCountryIt.atEnd()) {
                        var country = tempCountryIt.get();                       
                        bottomBar.remove(country);
                        issuingOffice.remove(country);
                        if (country != producingOffice) {
                            removeCountry(country);
                        }
                    }


                // below code is to check if any country has been deselected               
                dojo.forEach(countryList, function(country){
                    if (country != '' && country != undefined) {
                        if (!issuingOffice.contains(country) && !removedCoutries.contains(country)) {
                            if(producingOffice != country){
                                issuingOffice.add(country);
                                // TODO: check if country exists only then add 
                                if (!bottomBar.contains(country)) {
                                    bottomBar.add(country);
                                }
                                if (hashMap.containsKey(country)) {
                                    var graphic = hashMap.item(country);
                                    graphic.setSymbol(issuingOfficeSymbol);
                                    //map.graphics.add(graphic);
                                }   
                            }else if(producingOffice == country){
                                issuingOffice.add(country);
                                if (!bottomBar.contains(country)) {
                                    bottomBar.add(country);
                                }
                            }else{                              
                            }
                        }
                    }
                });
        }                            

}

Answer 1

From a once over:

• This:
  

      /*
       *  semicolon sepearated list of countryName will be avaialabe via selection module
       *  we have to replace ; with , so that we iterate over the list
       */
      countryName = replaceAll(";", ",", countryName);     
  
      /*
       *  once we replace ; with , then we split the string with , to get
       *  list of string
       */
      var countryList = countryName.split(",");
  

  should be this:

  //CountryNames contains a semicolon separated list of country names
  var countryList = countryNames.split(';');
  

  Note that countryName -> countryNames since you are passing more often than not multiple country names.

• This:
  

      // below code is to check if any country has been deselected
      var it = selectedCountries.getIterator();
      while (!it.atEnd()) {              
          var country = it.get();               
          var isSelected = false;          
          dojoArray.some(countryList, function(selectedCountry){
                  return isSelected=(country == selectedCountry);
          })                          
          if (!isSelected) {
              tempCountry.add(country); // this is list of countris that has to be deleted on click on GEt Details
          }
      }
  

  could be ( by using filter which is both in standard JS and in Dojo ):

      tempCountry = dojoArray.filter( countryList, function( country ){
        return dojoArray.indexOf( selectedCountries, country ) == -1;  
      });
  

  Since you are talking about loop optimization, you might even consider to change selectedCountries to act like a hashMap (an object with each selected country as a property set to to true), this way you could return even faster like this:

      tempCountry = dojoArray.filter( countryList, function( country ){
        return !selectedCountries[country];  
      });
  

• Iterators just seem wrong in JavaScript, I would suggest to just use forEach
• if (country != '' && country != undefined) { -> if(country){
• Where do you declare var hashMap ?
• Avoid console.log in production code
• Avoid commented out code in production code

• If bottomBar was a hashMap, then this:

  if (!bottomBar.contains(country)) {
    bottomBar.add(country);
  }
  

  would be bottomBar[country] = true;

• There is a ton of copy pasted code in there, with the filter trick, you might not need to address it, but in general, you should be more diligent in factoring copy pasted code into functions.