Iterate JSON GIF collection array to HTML file

I’ve written a piece of code which utilizes a for loop to iterate this array in JavaScript. This is my first attempt and I wonder if there is an optimal way to do this.

Fiddle

JavaScript

var gifCollection = {
"gifs": [
  {
  "gifName" : "gif-One",
  "gifLink" : "http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg"
 },
  {
  "gifName" : "gif-Two",
  "gifLink" : "http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg"
 },
  {
  "gifName" : "gif-Three",
  "gifLink" : "http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg"
 },
  {
  "gifName" : "gif-Four",
  "gifLink" : "http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg"
 },
 {
  "gifName" : "gif-Five",
  "gifLink" : "http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg"
 },
 {
  "gifName" : "gif-Six",
  "gifLink" : "http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg"
 },
 {
  "gifName" : "gif-Seven",
  "gifLink" : "http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg"
 },
 {
  "gifName" : "gif-Eight",
  "gifLink" : "http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg"
 },
 {
  "gifName" : "gif-Nine",
  "gifLink" : "http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg"
 },
 {
  "gifName" : "gif-Ten",
  "gifLink" : "http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg"
  }
 ]
};

var gifLink = gifCollection.gifs;
var div = document.getElementById('#vids');

for (var i in gifLink) {
  var src = gifLink[i].gifLink;
  //alert(gifLink.gifLink);
  console.log(src);
  //div.innerHTML = div.innerHTML + '<img src="' + src + '" />';
  $("#vids").append('<div id="wid-3"><img src="' + src + '"/></div>');
}

HTML

<div class="container">
 <div class="video">
  <div id="vids" class="row">
  </div>
 </div>
</div>

There aren’t many things wrong. And the code itself is surprisingly short. But this will be a really long review.

The first thing that bothers me is that you talk about JSON, but you show an object. That’s fine, but you should show how you decode the JSON into that object. But, within the scope of this review, I’ll refer to it as an object, which is what was presented. Not to mention that all the links point to .jpgs.

Now, lets tackle the code!

Javascript:

The indentation of that object is really off. Not to mention that you have mixed quote styles.

---

Each gif has the following:

{
    "gifName" : "gif-One",
    "gifLink" : "http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg"
}

We know those are gifs, so, why repeat gif in every single property? Using just name and link is enough.

And you don’t even use the gifName anywhere! So, just drop it! And just to have there to say it is gifs, you can remove it and just use an array.

The whole object can be something like this:

var gifCollection = [
    'http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg',
    'http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg',
    ' ... '
];

Notice that you have to use " instead if you are using this in JSON! As I refered earlier, I would review it as an object.

---

Looking a little below, you have this line:

var div = document.getElementById('#vids');

Well, that won’t work. What you have there is a CSS selector. Fear not, just remove the #. But a few lines below you have this:

//div.innerHTML = div.innerHTML + '<img src="' + src + '" />';

It seems like you made an attempt, but it didn’t worked. Since these 2 lines aren’t doing anything, just remove them.

Anyway, you are using jQuery below!

---

Right after it, you have this block:

for (var i in gifLink) {
    var src = gifLink[i].gifLink;
    //alert(gifLink.gifLink);
    console.log(src);
    //div.innerHTML = div.innerHTML + '<img src="' + src + '" />';
    $("#vids").append('<div id="wid-3"><img src="' + src + '"/></div>');
}

These comments contain old code. Just remove them! They won’t kill you. And that console.log can go away as well, since it isn’t useful to make the code work. It may be in debug.

But wait! You have this line:

$("#vids").append('<div id="wid-3"><img src="' + src + '"/></div>');

Oh no! You are creating duplicated ids! They MUST be unique! Or that will be invalid HTML. Use a class instead. Also, don’t be afraid of using the id videos. No one will die over 2 bytes. And it is way more clean.

Since I’ve suggested to change the structure of your object, you have to use a different for loop. Applying the changes above explained, and fixing the quotes:

for (var i = 0, length = gifCollection.length; i < length; i++) {
    var gif = gifCollection[i];
    $('#videos').append('<div class="video"><img src="' + gif + '"/></div>');
}

I’ve kept the length there because it will speed up your code quite a lot. Speaking in ‘speeding up’, you are re-running $('#vids') per gif. Just store it into a variable. Like this:

var videos = $('#videos');
for (var i = 0, length = gifCollection.length; i < length; i++) {
    var gif = gifCollection[i];
    videos.append('<div class="video"><img src="' + gif + '"/></div>');
}

Now you won’t be running jQuery everytime and it will be way faster!

---

Putting your Javascript code all together:

var gifCollection = [
    "http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg",
    "http://buffalogrove.sat.iit.edu/thumb/turkey_karadeniz_region-t2.jpg",
    " ... "
];

var videos = $('#videos');
for (var i = 0, length = gifCollection.length; i < length; i++) {
    var gif = gifCollection[i];
    videos.append('<div class="video"><img src="' + gif + '"/></div>');
}

---

HTML:

The HTML is pretty basic, but there’s still plenty to improve. Lets look at it on it’s original state:

<div class="container">
    <div class="video">
        <div id="vids" class="row">
        </div>
    </div>
</div>

Alright, you have a .container which has a .video which has #vids inside? What? Videos inside a video?

What I would do:

1. Remove .video. It’s useless there.
2. Change the id to videos
3. Give the class row to the container.
   The container is the one that will control the width of your #videos.
   It makes sense to set it this way.
4. Closing videos in the same line
   Since you are using jQuery, later on you may want to use the pseudo-selector :empty, which won’t work if you have newlines in it.

Applying all the changes:

<div class="container row">
    <div id="videos"></div>
</div>

And yes, it works just like the old HTML.