JavaScript - Optimizing a two array.map loop [closed]
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
2
down vote
favorite
I have to iterate two arrays, if the iso
propery of the first array is equal to address.country
of the second array condition is verified, assign address
and slug
of the second array (this.concessions
) to the first array (this.countries
).
at the end, you need to have a new this.countries
array that contains theaddress
and slug
property (in addition to the properties he already had)
this.countries.map((element) =>
this.concessions.map((value) =>
if (element.iso === value.address.country)
element.address = value.address
element.slug = value.slug
)
)
How can I optimize this, and for this case what is the best iterable to use, for ..of
for example ?
javascript
closed as off-topic by Graipher, Sam Onela, t3chb0t, Mast, Vogel612⦠Jan 15 at 15:02
This question appears to be off-topic. The users who voted to close gave this specific reason:
- "Questions must involve real code that you own or maintain. Pseudocode, hypothetical code, or stub code should be replaced by a concrete implementation. Questions seeking an explanation of someone else's code are also off-topic." â Graipher, Sam Onela, t3chb0t
add a comment |Â
up vote
2
down vote
favorite
I have to iterate two arrays, if the iso
propery of the first array is equal to address.country
of the second array condition is verified, assign address
and slug
of the second array (this.concessions
) to the first array (this.countries
).
at the end, you need to have a new this.countries
array that contains theaddress
and slug
property (in addition to the properties he already had)
this.countries.map((element) =>
this.concessions.map((value) =>
if (element.iso === value.address.country)
element.address = value.address
element.slug = value.slug
)
)
How can I optimize this, and for this case what is the best iterable to use, for ..of
for example ?
javascript
closed as off-topic by Graipher, Sam Onela, t3chb0t, Mast, Vogel612⦠Jan 15 at 15:02
This question appears to be off-topic. The users who voted to close gave this specific reason:
- "Questions must involve real code that you own or maintain. Pseudocode, hypothetical code, or stub code should be replaced by a concrete implementation. Questions seeking an explanation of someone else's code are also off-topic." â Graipher, Sam Onela, t3chb0t
If you aren't using the result ofmap
you should really be usingforEach
or a plain for loop (as per A Bravo Dev`s second example.
â Marc Rohloff
Jan 12 at 20:08
1
please give us samples of the two arrays, would be helpful
â JohnPan
Jan 13 at 16:02
Without knowing what the data looks like, we can't tell you what the best solution would look like.
â Mast
Jan 14 at 20:33
add a comment |Â
up vote
2
down vote
favorite
up vote
2
down vote
favorite
I have to iterate two arrays, if the iso
propery of the first array is equal to address.country
of the second array condition is verified, assign address
and slug
of the second array (this.concessions
) to the first array (this.countries
).
at the end, you need to have a new this.countries
array that contains theaddress
and slug
property (in addition to the properties he already had)
this.countries.map((element) =>
this.concessions.map((value) =>
if (element.iso === value.address.country)
element.address = value.address
element.slug = value.slug
)
)
How can I optimize this, and for this case what is the best iterable to use, for ..of
for example ?
javascript
I have to iterate two arrays, if the iso
propery of the first array is equal to address.country
of the second array condition is verified, assign address
and slug
of the second array (this.concessions
) to the first array (this.countries
).
at the end, you need to have a new this.countries
array that contains theaddress
and slug
property (in addition to the properties he already had)
this.countries.map((element) =>
this.concessions.map((value) =>
if (element.iso === value.address.country)
element.address = value.address
element.slug = value.slug
)
)
How can I optimize this, and for this case what is the best iterable to use, for ..of
for example ?
javascript
edited Jan 12 at 12:28
asked Jan 12 at 12:22
Mouad Ennaciri
486
486
closed as off-topic by Graipher, Sam Onela, t3chb0t, Mast, Vogel612⦠Jan 15 at 15:02
This question appears to be off-topic. The users who voted to close gave this specific reason:
- "Questions must involve real code that you own or maintain. Pseudocode, hypothetical code, or stub code should be replaced by a concrete implementation. Questions seeking an explanation of someone else's code are also off-topic." â Graipher, Sam Onela, t3chb0t
closed as off-topic by Graipher, Sam Onela, t3chb0t, Mast, Vogel612⦠Jan 15 at 15:02
This question appears to be off-topic. The users who voted to close gave this specific reason:
- "Questions must involve real code that you own or maintain. Pseudocode, hypothetical code, or stub code should be replaced by a concrete implementation. Questions seeking an explanation of someone else's code are also off-topic." â Graipher, Sam Onela, t3chb0t
If you aren't using the result ofmap
you should really be usingforEach
or a plain for loop (as per A Bravo Dev`s second example.
â Marc Rohloff
Jan 12 at 20:08
1
please give us samples of the two arrays, would be helpful
â JohnPan
Jan 13 at 16:02
Without knowing what the data looks like, we can't tell you what the best solution would look like.
â Mast
Jan 14 at 20:33
add a comment |Â
If you aren't using the result ofmap
you should really be usingforEach
or a plain for loop (as per A Bravo Dev`s second example.
â Marc Rohloff
Jan 12 at 20:08
1
please give us samples of the two arrays, would be helpful
â JohnPan
Jan 13 at 16:02
Without knowing what the data looks like, we can't tell you what the best solution would look like.
â Mast
Jan 14 at 20:33
If you aren't using the result of
map
you should really be using forEach
or a plain for loop (as per A Bravo Dev`s second example.â Marc Rohloff
Jan 12 at 20:08
If you aren't using the result of
map
you should really be using forEach
or a plain for loop (as per A Bravo Dev`s second example.â Marc Rohloff
Jan 12 at 20:08
1
1
please give us samples of the two arrays, would be helpful
â JohnPan
Jan 13 at 16:02
please give us samples of the two arrays, would be helpful
â JohnPan
Jan 13 at 16:02
Without knowing what the data looks like, we can't tell you what the best solution would look like.
â Mast
Jan 14 at 20:33
Without knowing what the data looks like, we can't tell you what the best solution would look like.
â Mast
Jan 14 at 20:33
add a comment |Â
3 Answers
3
active
oldest
votes
up vote
1
down vote
accepted
For performance use a for ... of
loop, it has much less overhead as it does not require a new context and heap allocation for each iteration that are required when you use the array iteration methods like Array.forEach
.
Also replacing the array using Array.map
requires two copies of the array to exist at the same time, the new array being generated by Array.map
and the existing array, on top of the heap allocations that Array.map
requires.
So the most efficient way is
for(const country of this.countries)
for(const conc of concessions)
if(conc.address.country === country.iso)
country.address = conc.address;
country.slug = conc.slug;
break; // only set first occurrence
You could also use a Map
to lookup the iso
. Though a little more memory usage it will be somewhat quicker overall if you maintained the concessions
iso
map alongside the concessions
array.
// The map needs to be created.
// but ideally you would maintain the map alongside code that maintains concessions
const concessionsIsoMap = new Map();
for(const conc of concessions)
concessionsIsoMap.set(conc.address.country, conc);
for(const country of this.countries)
// lookup for concessions.address.country is very fast as it uses
// a hash table to find the correct item, if any.
const concession = concessionsIsoMap.get(country.iso);
if(concession)
country.address = conc.address;
country.slug = conc.slug;
add a comment |Â
up vote
2
down vote
That'd be with map:
this.countries = this.countries.map(country =>
var concession = concessions.find(x => x.address.country === country.iso);
country.address = concession.address;
country.slug = concession.slug;
return country;
);
That'd be with forEach:
this.countries.forEach(country =>
var concession = concessions.find(x => x.address.country === country.iso);
country.address = concession.address;
country.slug = concession.slug;
);
I don't know if you can be sure there always be a concession for each country and how you should proceed in case there isn't.
add a comment |Â
up vote
0
down vote
Your algorithm is O(n2) because you're performing a linear search of concessions
for each element in countries
. If these arrays are large, it's better to create an object that maps countries directly to concessions. Object lookups are O(1), so the whole algorithm becomes O(n).
const countryMap = ;
this.concessions.forEach(c => countryMap[c.address.country] = c);
this.countries.forEach(country =>
var concession = countryMap[country.iso];
if (concession)
[country.address, country.slug] = [concession.address, concession.slug];
);
Is that not a little unsafe if Object`s prototype has been modified?
â Blindman67
Jan 12 at 23:15
Modified in what way? So that lookups aren't O(1)?
â Barmar
Jan 12 at 23:22
So there is no key clash. You should create a object used as a map viaconst countryMap = Object.create(null);
to remove any possible key clash.
â Blindman67
Jan 12 at 23:42
add a comment |Â
3 Answers
3
active
oldest
votes
3 Answers
3
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
1
down vote
accepted
For performance use a for ... of
loop, it has much less overhead as it does not require a new context and heap allocation for each iteration that are required when you use the array iteration methods like Array.forEach
.
Also replacing the array using Array.map
requires two copies of the array to exist at the same time, the new array being generated by Array.map
and the existing array, on top of the heap allocations that Array.map
requires.
So the most efficient way is
for(const country of this.countries)
for(const conc of concessions)
if(conc.address.country === country.iso)
country.address = conc.address;
country.slug = conc.slug;
break; // only set first occurrence
You could also use a Map
to lookup the iso
. Though a little more memory usage it will be somewhat quicker overall if you maintained the concessions
iso
map alongside the concessions
array.
// The map needs to be created.
// but ideally you would maintain the map alongside code that maintains concessions
const concessionsIsoMap = new Map();
for(const conc of concessions)
concessionsIsoMap.set(conc.address.country, conc);
for(const country of this.countries)
// lookup for concessions.address.country is very fast as it uses
// a hash table to find the correct item, if any.
const concession = concessionsIsoMap.get(country.iso);
if(concession)
country.address = conc.address;
country.slug = conc.slug;
add a comment |Â
up vote
1
down vote
accepted
For performance use a for ... of
loop, it has much less overhead as it does not require a new context and heap allocation for each iteration that are required when you use the array iteration methods like Array.forEach
.
Also replacing the array using Array.map
requires two copies of the array to exist at the same time, the new array being generated by Array.map
and the existing array, on top of the heap allocations that Array.map
requires.
So the most efficient way is
for(const country of this.countries)
for(const conc of concessions)
if(conc.address.country === country.iso)
country.address = conc.address;
country.slug = conc.slug;
break; // only set first occurrence
You could also use a Map
to lookup the iso
. Though a little more memory usage it will be somewhat quicker overall if you maintained the concessions
iso
map alongside the concessions
array.
// The map needs to be created.
// but ideally you would maintain the map alongside code that maintains concessions
const concessionsIsoMap = new Map();
for(const conc of concessions)
concessionsIsoMap.set(conc.address.country, conc);
for(const country of this.countries)
// lookup for concessions.address.country is very fast as it uses
// a hash table to find the correct item, if any.
const concession = concessionsIsoMap.get(country.iso);
if(concession)
country.address = conc.address;
country.slug = conc.slug;
add a comment |Â
up vote
1
down vote
accepted
up vote
1
down vote
accepted
For performance use a for ... of
loop, it has much less overhead as it does not require a new context and heap allocation for each iteration that are required when you use the array iteration methods like Array.forEach
.
Also replacing the array using Array.map
requires two copies of the array to exist at the same time, the new array being generated by Array.map
and the existing array, on top of the heap allocations that Array.map
requires.
So the most efficient way is
for(const country of this.countries)
for(const conc of concessions)
if(conc.address.country === country.iso)
country.address = conc.address;
country.slug = conc.slug;
break; // only set first occurrence
You could also use a Map
to lookup the iso
. Though a little more memory usage it will be somewhat quicker overall if you maintained the concessions
iso
map alongside the concessions
array.
// The map needs to be created.
// but ideally you would maintain the map alongside code that maintains concessions
const concessionsIsoMap = new Map();
for(const conc of concessions)
concessionsIsoMap.set(conc.address.country, conc);
for(const country of this.countries)
// lookup for concessions.address.country is very fast as it uses
// a hash table to find the correct item, if any.
const concession = concessionsIsoMap.get(country.iso);
if(concession)
country.address = conc.address;
country.slug = conc.slug;
For performance use a for ... of
loop, it has much less overhead as it does not require a new context and heap allocation for each iteration that are required when you use the array iteration methods like Array.forEach
.
Also replacing the array using Array.map
requires two copies of the array to exist at the same time, the new array being generated by Array.map
and the existing array, on top of the heap allocations that Array.map
requires.
So the most efficient way is
for(const country of this.countries)
for(const conc of concessions)
if(conc.address.country === country.iso)
country.address = conc.address;
country.slug = conc.slug;
break; // only set first occurrence
You could also use a Map
to lookup the iso
. Though a little more memory usage it will be somewhat quicker overall if you maintained the concessions
iso
map alongside the concessions
array.
// The map needs to be created.
// but ideally you would maintain the map alongside code that maintains concessions
const concessionsIsoMap = new Map();
for(const conc of concessions)
concessionsIsoMap.set(conc.address.country, conc);
for(const country of this.countries)
// lookup for concessions.address.country is very fast as it uses
// a hash table to find the correct item, if any.
const concession = concessionsIsoMap.get(country.iso);
if(concession)
country.address = conc.address;
country.slug = conc.slug;
answered Jan 12 at 23:11
Blindman67
5,4001320
5,4001320
add a comment |Â
add a comment |Â
up vote
2
down vote
That'd be with map:
this.countries = this.countries.map(country =>
var concession = concessions.find(x => x.address.country === country.iso);
country.address = concession.address;
country.slug = concession.slug;
return country;
);
That'd be with forEach:
this.countries.forEach(country =>
var concession = concessions.find(x => x.address.country === country.iso);
country.address = concession.address;
country.slug = concession.slug;
);
I don't know if you can be sure there always be a concession for each country and how you should proceed in case there isn't.
add a comment |Â
up vote
2
down vote
That'd be with map:
this.countries = this.countries.map(country =>
var concession = concessions.find(x => x.address.country === country.iso);
country.address = concession.address;
country.slug = concession.slug;
return country;
);
That'd be with forEach:
this.countries.forEach(country =>
var concession = concessions.find(x => x.address.country === country.iso);
country.address = concession.address;
country.slug = concession.slug;
);
I don't know if you can be sure there always be a concession for each country and how you should proceed in case there isn't.
add a comment |Â
up vote
2
down vote
up vote
2
down vote
That'd be with map:
this.countries = this.countries.map(country =>
var concession = concessions.find(x => x.address.country === country.iso);
country.address = concession.address;
country.slug = concession.slug;
return country;
);
That'd be with forEach:
this.countries.forEach(country =>
var concession = concessions.find(x => x.address.country === country.iso);
country.address = concession.address;
country.slug = concession.slug;
);
I don't know if you can be sure there always be a concession for each country and how you should proceed in case there isn't.
That'd be with map:
this.countries = this.countries.map(country =>
var concession = concessions.find(x => x.address.country === country.iso);
country.address = concession.address;
country.slug = concession.slug;
return country;
);
That'd be with forEach:
this.countries.forEach(country =>
var concession = concessions.find(x => x.address.country === country.iso);
country.address = concession.address;
country.slug = concession.slug;
);
I don't know if you can be sure there always be a concession for each country and how you should proceed in case there isn't.
answered Jan 12 at 17:58
A Bravo Dev
539110
539110
add a comment |Â
add a comment |Â
up vote
0
down vote
Your algorithm is O(n2) because you're performing a linear search of concessions
for each element in countries
. If these arrays are large, it's better to create an object that maps countries directly to concessions. Object lookups are O(1), so the whole algorithm becomes O(n).
const countryMap = ;
this.concessions.forEach(c => countryMap[c.address.country] = c);
this.countries.forEach(country =>
var concession = countryMap[country.iso];
if (concession)
[country.address, country.slug] = [concession.address, concession.slug];
);
Is that not a little unsafe if Object`s prototype has been modified?
â Blindman67
Jan 12 at 23:15
Modified in what way? So that lookups aren't O(1)?
â Barmar
Jan 12 at 23:22
So there is no key clash. You should create a object used as a map viaconst countryMap = Object.create(null);
to remove any possible key clash.
â Blindman67
Jan 12 at 23:42
add a comment |Â
up vote
0
down vote
Your algorithm is O(n2) because you're performing a linear search of concessions
for each element in countries
. If these arrays are large, it's better to create an object that maps countries directly to concessions. Object lookups are O(1), so the whole algorithm becomes O(n).
const countryMap = ;
this.concessions.forEach(c => countryMap[c.address.country] = c);
this.countries.forEach(country =>
var concession = countryMap[country.iso];
if (concession)
[country.address, country.slug] = [concession.address, concession.slug];
);
Is that not a little unsafe if Object`s prototype has been modified?
â Blindman67
Jan 12 at 23:15
Modified in what way? So that lookups aren't O(1)?
â Barmar
Jan 12 at 23:22
So there is no key clash. You should create a object used as a map viaconst countryMap = Object.create(null);
to remove any possible key clash.
â Blindman67
Jan 12 at 23:42
add a comment |Â
up vote
0
down vote
up vote
0
down vote
Your algorithm is O(n2) because you're performing a linear search of concessions
for each element in countries
. If these arrays are large, it's better to create an object that maps countries directly to concessions. Object lookups are O(1), so the whole algorithm becomes O(n).
const countryMap = ;
this.concessions.forEach(c => countryMap[c.address.country] = c);
this.countries.forEach(country =>
var concession = countryMap[country.iso];
if (concession)
[country.address, country.slug] = [concession.address, concession.slug];
);
Your algorithm is O(n2) because you're performing a linear search of concessions
for each element in countries
. If these arrays are large, it's better to create an object that maps countries directly to concessions. Object lookups are O(1), so the whole algorithm becomes O(n).
const countryMap = ;
this.concessions.forEach(c => countryMap[c.address.country] = c);
this.countries.forEach(country =>
var concession = countryMap[country.iso];
if (concession)
[country.address, country.slug] = [concession.address, concession.slug];
);
answered Jan 12 at 22:58
Barmar
27019
27019
Is that not a little unsafe if Object`s prototype has been modified?
â Blindman67
Jan 12 at 23:15
Modified in what way? So that lookups aren't O(1)?
â Barmar
Jan 12 at 23:22
So there is no key clash. You should create a object used as a map viaconst countryMap = Object.create(null);
to remove any possible key clash.
â Blindman67
Jan 12 at 23:42
add a comment |Â
Is that not a little unsafe if Object`s prototype has been modified?
â Blindman67
Jan 12 at 23:15
Modified in what way? So that lookups aren't O(1)?
â Barmar
Jan 12 at 23:22
So there is no key clash. You should create a object used as a map viaconst countryMap = Object.create(null);
to remove any possible key clash.
â Blindman67
Jan 12 at 23:42
Is that not a little unsafe if Object`s prototype has been modified?
â Blindman67
Jan 12 at 23:15
Is that not a little unsafe if Object`s prototype has been modified?
â Blindman67
Jan 12 at 23:15
Modified in what way? So that lookups aren't O(1)?
â Barmar
Jan 12 at 23:22
Modified in what way? So that lookups aren't O(1)?
â Barmar
Jan 12 at 23:22
So there is no key clash. You should create a object used as a map via
const countryMap = Object.create(null);
to remove any possible key clash.â Blindman67
Jan 12 at 23:42
So there is no key clash. You should create a object used as a map via
const countryMap = Object.create(null);
to remove any possible key clash.â Blindman67
Jan 12 at 23:42
add a comment |Â
If you aren't using the result of
map
you should really be usingforEach
or a plain for loop (as per A Bravo Dev`s second example.â Marc Rohloff
Jan 12 at 20:08
1
please give us samples of the two arrays, would be helpful
â JohnPan
Jan 13 at 16:02
Without knowing what the data looks like, we can't tell you what the best solution would look like.
â Mast
Jan 14 at 20:33