HiveBrain v1.2.0
Get Started
← Back to all entries
principlejavascriptMinor

A more efficient approach to existing JavaScript coding

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codingjavascriptmoreefficientexistingapproach

Problem

I am now working on a Node.js project, and on my ejs view page, I'm retrieving this array of objects, like:

[{ name: 'Andy',
   age: 20,
   username: 'andygoodluck'},
  {name: 'Terry',
   age: 21,
   username: 'terrygoodnight'}]


My objective is to make use of this data to dynamically create and inject content using JavaScript. My existing approach is:

  • Put this data into a variable, say usersObjArr.



  • Set number of user blocks to be shown per row with a variable, say numUsersPerRow=3.



  • Calculate the number of rows I will need, like numRows=getNumRows(usersObjArr.length);



-
If there is only one row of content needed, simply go through the array of objects and push them into the row:

for(var j=0;j<usersObjArr;j++){
  insertUserBlock();//pushes user objects one by one into the row
}


-
Else if there is more than one row, check that I am not working with the last row. As long as it is not the last row, insert the predetermined number of user blocks using the array of objects. Like:

for(var i=0;i<numRows;i++){
  if(i!=(numRows-1)){
  //as long as it's not the last row, insert 3 users
  for(var k=0;k<numUsersPerRow;k++){
      insertUserBlock(i); //insert 3 blocks into the row specified by the argument
  }


-
On the other hand, if it is the last row, then I will insert the number of users equivalent to the remainder:

else{
    for(var k=0;k<usersObjArr.length%numGamesPerRow;k++){
        insertUserBlock(i);
    }
}


I can't help but feel that this is not the most efficient approach. I am a web designer turned frontend developer and have a weak programming background, so I can really do with some advice on how to improving my coding above.

==========UPDATED==========

Here's the full code:

```
function getNumRows(numOfUsers){
return Math.ceil(numOfUsers/3);
}

function insertUserBlocks(usersObjArr){
var numUsersAdded=0,
numUsersPerRow=3,
numRows=getNumRows(usersObjArr.length);
if(usersObjArr.length')

Solution

function createUserList(users) {
    var $table = $('');
    for (var i = 0; i ').appendTo($table);
        }
        $row.append(createUserBlock(users[i]));
    }
    return $table;
}

function createUserBlock(user) {
    return $('', {'class': 'user-block', 'text': user.name});
}

$('#main').append(createUserList(users));


Instead of querying and updating elements in the document with $('#row'+rowNum+' .span4 div') try to build a detached fragment, and insert it as one whole. The difference in speed is very noticeable on large data sets.

Try to access elements through variables. You can create a row and then store it in a variable $row, and then access it as $row.append(...). This is both easier to read and faster than $('#row'+rowNum+' .span4').append(...).

''+userNickname+'' - there are several reasons why you shouldn't concatenate unescaped plain text with HTML, but the main reason is cross-site scripting. Imagine if some user's nickname is location = 'http://pwned.com/'. You probably don't want to allow someone to redirect your visitors to another site, or create fake login forms, or steal cookies. Here's how you can avoid that: $('').text(userName)

Code Snippets

function createUserList(users) {
    var $table = $('<div class="user-table">');
    for (var i = 0; i < users.length; i++) {
        if (i % 3 === 0) {
            var $row = $('<div class="row">').appendTo($table);
        }
        $row.append(createUserBlock(users[i]));
    }
    return $table;
}

function createUserBlock(user) {
    return $('<div>', {'class': 'user-block', 'text': user.name});
}

$('#main').append(createUserList(users));

Context

StackExchange Code Review Q#36948, answer score: 2

Revisions (0)

No revisions yet.