2
\$\begingroup\$

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:

enter image description here

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Mar 18, 2016 at 14:13
\$\endgroup\$
2
  • \$\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\$ Commented Mar 18, 2016 at 14:34
  • \$\begingroup\$ @Sumurai8 look at my EDIT part and the image I pasted. \$\endgroup\$ Commented Mar 18, 2016 at 14:37

2 Answers 2

2
\$\begingroup\$

There is A LOT of code you have going on in multiple forEach loops. Need to partition out tasks.

  1. Have a generic Function to Generate Cards
  2. 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

JS Bin

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;
 }
answered Mar 18, 2016 at 18:48
\$\endgroup\$
2
  • \$\begingroup\$ for how long have you been programming? It took around an hour to understand it. Looks awesome and clean, thanks :) \$\endgroup\$ Commented 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\$ Commented Mar 18, 2016 at 20:37
1
\$\begingroup\$

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.map

Map maps the current value of an Array to the new value by calling a specified function reference with the current value. (mdn)

Array.prototype.reduce

Reduce 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
}
answered Mar 18, 2016 at 18:48
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.