HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Highlight countries based on category

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
countriescategoryhighlightbased

Problem

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/symbo

Solution

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.

Code Snippets

/*
     *  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(",");
//CountryNames contains a semicolon separated list of country names
var countryList = countryNames.split(';');
// 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
        }
    }
tempCountry = dojoArray.filter( countryList, function( country ){
      return dojoArray.indexOf( selectedCountries, country ) == -1;  
    });
tempCountry = dojoArray.filter( countryList, function( country ){
      return !selectedCountries[country];  
    });

Context

StackExchange Code Review Q#42873, answer score: 4

Revisions (0)

No revisions yet.