patternjavascriptMinor
Highlight countries based on category
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
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:
Below is the code. Please validate:
```
var issuingOffice,selectedCountries,issuingOfficeSymbol,mapDefaultSymbol,selectedOfficeSymbol;
require(["dojox/collections/ArrayList","esri/symbo
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 whichI 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,RussiaBelow is the code. Please validate:
```
var issuingOffice,selectedCountries,issuingOfficeSymbol,mapDefaultSymbol,selectedOfficeSymbol;
require(["dojox/collections/ArrayList","esri/symbo
Solution
From a once over:
-
This:
should be this:
Note that
-
This:
could be ( by using
Since you are talking about loop optimization, you might even consider to change
-
If
would be
-
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.login 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
filtertrick, 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.