I am going deep on this array in order to make an id
comparison. I want to know if there is a way to use like a kind of flatten function to avoid using _.each
the way I am using it because I think it is not a good practice.
.json file:
[
{
"enterprise" : [
{
"id" : "743512",
"name" : "Hellen Quesada",
"role" : "P. Software Engineer",
"picture" : "http://i276.photobucket.com/albums/kk35/Sicable/200x200/06.png",
"skype" : "he.quesada",
"email" : "[email protected]",
"account" : "Digitas, Flag",
"subDept" : "Enterprise",
"location" : "Offshore: Costa Rica"
},
{
"id" : "743587",
"name" : "Jonathan Chavez",
"role" : "P. Creative Engineer",
"picture" : "http://i276.photobucket.com/albums/kk35/Sicable/200x200/06.png",
"skype" : "jonathanchavesb",
"email" : "[email protected]",
"account" : "Digitas, Flag",
"subDept" : "Enterprise",
"location" : "Offshore: Costa Rica"
},
{
"id" : "749058",
"name" : "Rodrigo Ovares",
"role" : "Creative Engineer",
"picture" : "http://i276.photobucket.com/albums/kk35/Sicable/200x200/06.png",
"skype" : "rodrigo.ovares.arroyo",
"email" : "[email protected]",
"account" : "Digitas, Flag",
"subDept" : "Enterprise",
"location" : "Offshore: Costa Rica"
},
{
"id" : "750684",
"name" : "Juan Morera",
"role" : "Software Engineer",
"picture" : "http://i276.photobucket.com/albums/kk35/Sicable/200x200/06.png",
"skype" : "morerajuan",
"email" : "[email protected]",
"account" : "Digitas, Flag",
"subDept" : "Enterprise",
"location" : "Offshore: Costa Rica"
}
]
},
"consumer" : [
{
"id" : "743360",
"name" : "Leroy Bernard",
"role" : "Sr. Software Engineer",
"picture" : "http://i276.photobucket.com/albums/kk35/Sicable/200x200/06.png",
"skype" : "leroy.bernard.cr",
"email" : "[email protected]",
"account" : "Digitas, Flag",
"subDept" : "Consumer",
"location" : "Offshore: Costa Rica"
},
{
"id" : "765434",
"name" : "Helga Martinez",
"role" : "Production Manager",
"picture" : "http://i276.photobucket.com/albums/kk35/Sicable/200x200/06.png",
"skype" : "helga.martinez.cr",
"email" : "[email protected]",
"account" : "Digitas, Flag",
"subDept" : "Consumer",
"location" : "Offshore: Costa Rica"
},
{
"id" : "764954",
"name" : "Marvin Solano",
"role" : "Sr. Frontend Engineer",
"picture" : "http://i276.photobucket.com/albums/kk35/Sicable/200x200/06.png",
"skype" : "mase9207",
"email" : "[email protected]",
"account" : "Digitas, Flag",
"subDept" : "Consumer",
"location" : "Offshore: Costa Rica"
}
]
}
]
I am calling this function every time I need to compare an id
:
var compareId = function(id, evt) {
$('.spinner, .while-loads').fadeIn();
var cardTemplate = [],
getData = localStorage.getItem('jsonData'),
json = JSON.parse(getData);
_.each(json, function(items) {
_.each(items, function(item) {
_.each(item, function(data) {
setTimeout(function() {
if (id === data.id) {
$('.removeLi').remove();
$('.spinner, .while-loads').fadeOut();
cardTemplate.push('<li><span class="name-role" id="name">' +
'<span><img class="card-picture" src="'+ data.picture +'"/></span>' +
'<div class="main-info-card">' +
data.name + '<br>' +
data.role + '<br>' +
'Employee N.' + data.id +
'</div>' +
'</span></li>' +
'<li><p>Email: <strong>'+ data.email +'</strong></p></li>' +
'<li><p>Skype: <strong>'+ data.skype +'</strong></p></li>' +
'<li><p class="team"> '+ data.account +' <br> <strong>Enterprise</strong></p></li>' +
'<li><strong> '+ data.location +'</strong></li>');
$('<ul/>', {
"class" : "removeLi",
html: cardTemplate.join('')
}).prependTo('.personal-info');
if ($('.removeLi').length) {
$('#sidebar li')
.first()
.css({'background': cardColor,
'color': fontColor
});
}
if ($('.removeLi').length > 1 ) {
$('.removeLi').remove();
}
}
}, 1000);
});
});
});
};
The result in the view:
-
\$\begingroup\$ I would like to see more of the code. In particular something that explains why you choose to use a timeout, and if you are calling this every time for a particular id. \$\endgroup\$Sumurai8– Sumurai82016年03月18日 14:34:26 +00:00Commented Mar 18, 2016 at 14:34
-
\$\begingroup\$ @Sumurai8 look at my EDIT part and the image I pasted. \$\endgroup\$Non– Non2016年03月18日 14:37:26 +00:00Commented Mar 18, 2016 at 14:37
2 Answers 2
There is A LOT of code you have going on in multiple forEach loops. Need to partition out tasks.
- Have a generic Function to Generate Cards
- Have a Function to find users
Then combine them. This will help with DRY (Don't repeat yourself). So these can be reused on other parts of the application
Here is a method to find the user based on Id, Im sure this could be cleaned and refactored more
function createCard(value, cb){
var result;
_.forEach(data, function(items){
var results =_.filter(items[Object.keys(items)[0]], {'id' : value});
if (results.length !== 0) result = results[0];
});
return _.isFunction(cb) ? cb(result) : result;
}
-
\$\begingroup\$ for how long have you been programming? It took around an hour to understand it. Looks awesome and clean, thanks :) \$\endgroup\$Non– Non2016年03月18日 20:08:13 +00:00Commented Mar 18, 2016 at 20:08
-
\$\begingroup\$ About 10 years, Thank you. Also don't use timeouts. Implement Promises. They won't drag your response time down like a setTimeout will. \$\endgroup\$Derek S– Derek S2016年03月18日 20:37:20 +00:00Commented Mar 18, 2016 at 20:37
Naming, styling
Your function compareId
does not name what the function actually does (show a popup card with information).
Variable declarations in javascript are hoisted to the top of their context. I recommend putting them there for clarity.
You are using a single var
, and declare multiple variables using commas. I recommend against this, because omitting one comma, or having a semicolon will drop part of your variables in the global scope. It's easy to make such a mistake while adding or removing variables. Instead put var
in front of every variable and close all statements with a ;
.
Potential bug
Since your code contains no comments, it is impossible for me to deduct what you want to happen in certain edge cases. Weirdly enough you have a piece of code that removes all results if you have more than one result. This is somewhat odd behaviour. Did you mean to remove everything except the first occurance of this class instead?
Performance
You are doing the same work over and over if you call compareId
multiple times. You cannot get around doing all the work once, but for any subsequent calls you can already have prepared an intermediate data structure.
To do this, you can use two native functions:
Array.prototype.mapMap maps the current value of an Array to the new value by calling a specified function reference with the current value. (mdn)
Array.prototype.reduceReduce reduces an Array to a single value. Note that this single value can be anything, including an Array. (mdn)
Possible implementation (in pseudo code)var showCard = function(id) {
var data;
var credentials;
if(this.processedData === undefined) {
data = JSON.parse( ... );
this.processedData = data.reduce(function(prev, cur) {
}, []);
}
credentials = this.processedData[id];
if( !credentials ) {
//blah
} else {
//make html
//put it in there
}
//--spinner
}