1
\$\begingroup\$

At the moment, it gets the first page worth of data and then each subsequent page when the user scrolls down to the bottom of the page.

My JavaScript/jQuery level is fairly weak, however I have managed to cobble this together and would like to know how I can improve my code.

 var page = 1;

 $(function() {
   getUsers(page);
   page++;
 });


 $(window).scroll(function() {
   if ($(window).scrollTop() + $(window).height() == $(document).height()) {
     getUsers(page);
     page++;
   }
 });

 function getUsers(page) {

   $.ajax({
     url: '/ajax/users?page=' + page
   }).done(function(data) {
     var users = data;
     var user;
     var url = "<?php echo url('profile/') ?>";

     if (page > users.last_page) {
       return true;
     } else {
       $.each(users.data, function(key, value) {
         var image;
         if (value.image[0]) {
           image = value.image[0].name;
         } else {
           if (value.profile.gender == 'male') {
             image = 'maleDefault.png';
           } else {
             image = 'femaleDefault.jpg';
           }
         }
         user = "<div class='col-xs-6 col-md-3'> <a href='" + url + "/" + value.username + "' class='thumbnail'><img src='http://localhost:8000/image/" + image + "'" +
           "class='img-rounded' alt='' width='196.372' height='159.368'></a></div>";
         $('.panel-body').append(user);
       });
     }
   });
 }

\$\endgroup\$
3
\$\begingroup\$

When you have an if/else statement inside an else statement and that is the only thing inside that statement, you should make the else statement into and else if instead. So you would change this:

     if (value.image[0]) {
       image = value.image[0].name;
     } else {
       if (value.profile.gender == 'male') {
         image = 'maleDefault.png';
       } else {
         image = 'femaleDefault.jpg';
       }
     }

to this

if (value.image[0]) {
   image = value.image[0].name;
} else if (value.profile.gender == 'male') {
     image = 'maleDefault.png';
} else {
     image = 'femaleDefault.jpg';
}

All that this does is remove a level of indentation to make the code easier to read.

\$\endgroup\$
2
\$\begingroup\$

As @Malachi points out, your indentation is off, and you really ought to fix that (Four spaces instead of two)


You could replace the following with a nice little ternary statement, which follow the structure:

variable == variableToCompare ? doSomethingIfTrue() : doSomethingIfFalse();
     if (value.image[0]) {
       image = value.image[0].name;
     } else {
       if (value.profile.gender == 'male') {
         image = 'maleDefault.png';
       } else {
         image = 'femaleDefault.jpg';
       }
     }

Which could become:

if (value.image[0]) {
    image = value.image[0].name;
} else {
    image = (value.profile.gender != 'male' ? 'fe' : '') + 'maleDefault.png';
}

Or even (if you're daring):

image = (value.image[0] ? value.image[0].name :
            (value.profile.gender != 'male' ? 'fe' : '') + 'maleDefault.png')
        );
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.