2
\$\begingroup\$

I've adapted an Instagram widget from this example so that it now appends 8 images to their respective IDs using an Instagram username. I want my widget implementation to be better and more efficient. I'm very new to this so I'm sure there are parts that are no longer needed or that could be greatly improved.

var accessToken = ''; // Access token
var instagram = function () {
  return {
    getUserIDByUsername: function (strUser) { // By username
      var getUserURL = 'https://api.instagram.com/v1/users/search?q=' + strUser + '&access_token=' + accessToken + ''
      $.ajax({
        type: "GET",
        dataType: "jsonp",
        cache: false,
        url: getUserURL,
        success: function (data) {
          var getUserID = data.data[0].id;
          instagram.getUserDetailsByUserID(getUserID);
        }
      });
    },
    getUserDetailsByUserID: function (userID) { // By userid
      $.ajax({
        type: "GET",
        dataType: "jsonp",
        cache: false,
        url: "https://api.instagram.com/v1/users/" + userID + "/?access_token=" + accessToken + "",
        success: function (data) {
          if (data.meta && data.meta.code == "200") {
            //console.log(data);
            instagram.loadImages(userID);
          } else if (data.meta && data.meta.code == "400" && data.meta.error_message) {
            console.log(data.meta.error_message);
          }
        }
      });
    },
    loadImages: function (userID) { // Get user photos
      var getImagesURL = "https://api.instagram.com/v1/users/" + userID + "/media/recent/?access_token=" + accessToken + ""
      $.ajax({
        type: "GET",
        dataType: "jsonp",
        cache: false,
        url: getImagesURL,
        success: function (data) {
          console.log(data);
          size = data.data[0].images.standard_resolution.url;
          $("#insta-1").append("<a target='_blank' href='" + data.data[0].link + "'><img src='" + size + "'></img></a>");
          size = data.data[1].images.standard_resolution.url;
          $("#insta-2").append("<a target='_blank' href='" + data.data[1].link + "'><img src='" + size + "'></img></a>");
          size = data.data[2].images.standard_resolution.url;
          $("#insta-3").append("<a target='_blank' href='" + data.data[2].link + "'><img src='" + size + "'></img></a>");
          size = data.data[3].images.standard_resolution.url;
          $("#insta-4").append("<a target='_blank' href='" + data.data[3].link + "'><img src='" + size + "'></img></a>");
          size = data.data[4].images.standard_resolution.url;
          $("#insta-5").append("<a target='_blank' href='" + data.data[4].link + "'><img src='" + size + "'></img></a>");
          size = data.data[5].images.standard_resolution.url;
          $("#insta-6").append("<a target='_blank' href='" + data.data[5].link + "'><img src='" + size + "'></img></a>");
          size = data.data[6].images.standard_resolution.url;
          $("#insta-7").append("<a target='_blank' href='" + data.data[6].link + "'><img src='" + size + "'></img></a>");
          size = data.data[7].images.standard_resolution.url;
          $("#insta-8").append("<a target='_blank' href='" + data.data[7].link + "'><img src='" + size + "'></img></a>");
        }
      });
    }
  }
}();
$(document).ready(function () {
  var strUser = "saturdaysnyc"; // Instagram username
  if (isNaN(strUser)) {
    instagram.getUserIDByUsername(strUser);
  } else {
    instagram.getUserDetailsByUserID(strUser);
  }
});
\$\endgroup\$
3
\$\begingroup\$

first thing one could spot is this duplications:

size = data.data[0].images.standard_resolution.url;
          $("#insta-1").append("<a target='_blank' href='" + data.data[0].link + "'><img src='" + size + "'></img></a>");
          size = data.data[1].images.standard_resolution.url;
          $("#insta-2").append("<a target='_blank' href='" + data.data[1].link + "'><img src='" + size + "'></img></a>");
          size = data.data[2].images.standard_resolution.url;
          $("#insta-3").append("<a target='_blank' href='" + data.data[2].link + "'><img src='" + size + "'></img></a>");
          size = data.data[3].images.standard_resolution.url;
          $("#insta-4").append("<a target='_blank' href='" + data.data[3].link + "'><img src='" + size + "'></img></a>");
          size = data.data[4].images.standard_resolution.url;
          $("#insta-5").append("<a target='_blank' href='" + data.data[4].link + "'><img src='" + size + "'></img></a>");
          size = data.data[5].images.standard_resolution.url;
          $("#insta-6").append("<a target='_blank' href='" + data.data[5].link + "'><img src='" + size + "'></img></a>");
          size = data.data[6].images.standard_resolution.url;
          $("#insta-7").append("<a target='_blank' href='" + data.data[6].link + "'><img src='" + size + "'></img></a>");
          size = data.data[7].images.standard_resolution.url;
          $("#insta-8").append("<a target='_blank' href='" + data.data[7].link + "'><img src='" + size + "'></img></a>");

One could change it to something more condensed like:

for(var i=0; i<9; i++) {
    size = data.data[i].images.standard_resolution.url;
    $("#insta-"+i).append("<a target='_blank' href='" + data.data[i].link + "'><img src='" + size + "'></img></a>")
}

Also creating new nodes multiple time is ineficient since browser will re-paint it every time, so you could change it to (I assume that #insta-X elements are just your way to hook it in some place in html, if that's the only reason you may ditch them and create ie. <div class="insta-container"></div>)

links = "";
for(var i=0; i<9; i++) {
    size = data.data[i].images.standard_resolution.url;
    links += "<a target='_blank' href='" + data.data[i].link + "'><img src='" + size + "'></img></a>";
}
$(".insta-container").append(links);

Also it looks like with size you accidentally declare global variable (window.size) - not a good idea - you need to make var size to make it local:

loadImages: function (userID) { // Get user photos
      var size, //declare local var, then use it as before
          getImagesURL = "https://api.instagram.com/v1/users/" + userID + "/media/recent/?access_token=" + accessToken + ""
      ;

Also you may want to read about hoisting to know those var ... declarations should be at the top of your function.

\$\endgroup\$
  • \$\begingroup\$ This is a great answer and thanks for the link regarding hoisting very helpful. \$\endgroup\$ – Thomas Taylor Jun 14 '14 at 0:15
  • \$\begingroup\$ Thank you :D Of course you may go beyond. For example when you want to create more complex html structures instead of concatenating strings better idea would be to use a template system or at least create nodes with jQuery help $("<a>", {href: "http://example.com}) but I can't write examples properly just from my head, but this is also something you can explore on your own. \$\endgroup\$ – Grzegorz Jun 14 '14 at 5:44
  • \$\begingroup\$ In it's current state I think a template system would be overkill. Regardless, you've been a great help thank you. \$\endgroup\$ – Thomas Taylor Jun 16 '14 at 1:01

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.