patternjavascriptMinor
Checking tag colors
Viewed 0 times
colorscheckingtag
Problem
I have an array
The goal is to efficiently check the
This is what I have so far:
Is there a better way to write this? Some sort of nested loop?
vs.tags which may contain hundreds of objects, and a smaller array coloredTags which only contains up to 3 objects.The goal is to efficiently check the
ids in coloredTags to the ids in vs.tags and perform some action.This is what I have so far:
var coloredTags = ColoredTagFactory.retrieveColoredTags();
console.log('coloredTags = ', coloredTags); // = 2
console.log('vs.tags = ', vs.tags); // = 25
var numColors = coloredTags.length;
for (var i=0; i<vs.tags.length; i++) {
if (numColors === 1) {
if (vs.tags[i].term_id === coloredTags[0].id) {
vs.tags[i].border1 = true;
}
}
else if (numColors === 2) {
if (vs.tags[i].term_id === coloredTags[1].id) {
vs.tags[i].border2 = true;
}
}
}Is there a better way to write this? Some sort of nested loop?
Solution
One method of rewriting it could be checking
You should also store
This should save you some performance, as
Another thing you could consider, which may be more beneficial, is to rewrite it to call a function when a match is found:
Then, in
Note: I'm not an angular.js guy, or a Javascript expert, so the above may need tweaking to work.
This should help separate responsibilities of your code, and make it easier to maintain.
if (numColors === 1) and likewise for 2 before you enter the loop.You should also store
coloredTags[#].id outside the loop.if (numColors === 1) {
var coloredTagId = coloredTags[numColors - 1].id;
for (var i=0; i<vs.tags.length; i++) {
if (vs.tags[i].term_id === coloredTagId) {
vs.tags[i].border1 = true;
}
}
}
else if (numColors === 2) {
var coloredTagId = coloredTags[numColors - 1].id;
for (var i=0; i<vs.tags.length; i++) {
if (vs.tags[i].term_id === coloredTagId) {
vs.tags[i].border2 = true;
}
}
}This should save you some performance, as
numColors cannot change in the loop so there's no need to check the value of it each iteration, and you grab the coloredTags[#].id value once instead of each iteration.Another thing you could consider, which may be more beneficial, is to rewrite it to call a function when a match is found:
if (numColors >= 1 && numColors <= 2) { // change 1/2 as needed
var coloredTagId = coloredTags[numColors - 1].id;
for (var i=0; i<vs.tags.length; i++) {
if (vs.tags[i].term_id === coloredTagId) {
someFunction(vs.tags[i], coloredTagId);
}
}
}Then, in
someFunction:function someFunction(vsTagsObject, coloredTagId) {
switch (coloredTagId) {
case 1:
vsTagsObject.border1 = true;
break;
case 2:
vsTagsObject.border2 = true;
break;
}
}Note: I'm not an angular.js guy, or a Javascript expert, so the above may need tweaking to work.
This should help separate responsibilities of your code, and make it easier to maintain.
Code Snippets
if (numColors === 1) {
var coloredTagId = coloredTags[numColors - 1].id;
for (var i=0; i<vs.tags.length; i++) {
if (vs.tags[i].term_id === coloredTagId) {
vs.tags[i].border1 = true;
}
}
}
else if (numColors === 2) {
var coloredTagId = coloredTags[numColors - 1].id;
for (var i=0; i<vs.tags.length; i++) {
if (vs.tags[i].term_id === coloredTagId) {
vs.tags[i].border2 = true;
}
}
}if (numColors >= 1 && numColors <= 2) { // change 1/2 as needed
var coloredTagId = coloredTags[numColors - 1].id;
for (var i=0; i<vs.tags.length; i++) {
if (vs.tags[i].term_id === coloredTagId) {
someFunction(vs.tags[i], coloredTagId);
}
}
}function someFunction(vsTagsObject, coloredTagId) {
switch (coloredTagId) {
case 1:
vsTagsObject.border1 = true;
break;
case 2:
vsTagsObject.border2 = true;
break;
}
}Context
StackExchange Code Review Q#101382, answer score: 2
Revisions (0)
No revisions yet.