0

I am adding some conditional logic to check for elements in an array and if they exist, adding classes using JS-generated mark-up to do so. Is there a way to add conditional logic in between two strings (which are outputted as markup from JS) to reduce some of this redundant conditional logic?

for (var i=0; i<data.length; i++) {
        var gallery = data[i];

        if (typeof gallery.showcase !== "undefined") {
            if (typeof gallery.video !== "undefined") {
                $(
                "<li class='video "  + (typeof gallery.video !== "undefined" ? "test ") + gallery.showcase + "'>" +
                    "<a href='" + gallery.link + "'></a>" +
                "</li>").appendTo("#gallery ul");   
            }
            else {
                $(
                "<li class='image " + gallery.showcase + "'>" +
                    "<a href='" + gallery.link + "'>" + 
                        "<img src=" + gallery.image + " alt='" + gallery.title + "'>" +
                    "</a>"
                + "</li>").appendTo("#gallery ul"); 
            }       
        }
        else {
            if (typeof gallery.video !== "undefined") {
                $(
                "<li class='video'>" +
                    "<a href='" + gallery.link + "'></a>" + 
                "</li>").appendTo("#gallery ul");   
            }
            else {
                $(
                "<li class='image'>" +
                    "<a href='" + gallery.link + "'>" + 
                        "<img src=" + gallery.image + " alt='" + gallery.title + "'>" +
                    "</a>"
                + "</li>").appendTo("#gallery ul"); 
            }       
        }
    }

What I am hoping for is to add classes to the JS-generated markup doing conditional checks in a much more cleaner manner. Thoughts?

Yasir
  • 879
  • 5
  • 13
  • 31
  • you should use jquery methods such as `.attr` and `.prop` to define attributes and properties on jquery objects, not string concatenation. – jbabey Jul 13 '12 at 18:06
  • http://minitechme.blogspot.ca/2012/01/stop-using-strings-for-html-and-embrace.html – Ry- Jul 13 '12 at 18:07
  • 2
    @Raminson the `typeof` unary operator returns the type as a string. this is perfectly valid and is best practice. – jbabey Jul 13 '12 at 18:08
  • @jbabey, why should we use `.attr` & `prop` in place of direct values in strings ? (if other than nice looking code ) – Jashwant Jul 13 '12 at 19:08
  • @Jashwant faster, cleaner, more maintainable: http://stackoverflow.com/questions/2202269/best-way-to-add-dom-elements-with-jquery – jbabey Jul 13 '12 at 19:16

3 Answers3

1

You have a lot of repetition - you can achieve what you need by identifying common strings or elements and refactoring the code somewhat.

for (var i = 0; i < data.length; i++) {
    var gallery = data[i];

    var isImage = (typeof gallery.video === 'undefined');

    var $li = $('<li>', {'class': isImage ? 'image' : 'video'});
    if (typeof gallery.showcase !== 'undefined') {
        $li.addClass(gallery.showcase);
    }

    var $a = $('<a>', {href: gallery.link});
    if (isImage) {
        $('<img>', {src: gallery.image, alt: gallery.title}).appendTo($a);
    }       

    $li.appendTo('#gallery ul').append($a);
}

I'm not 100% sure this is correct (since to be honest the original code is somewhat hard to parse) but it should demonstrate approximately how you would solve this.

Alnitak
  • 334,560
  • 70
  • 407
  • 495
0

you should use jquery methods such as .attr and .prop to define attributes and properties on jquery objects, not string concatenation.

Not only is this best practice, but it would eliminate your redundant code: (this is just an example, i realize it does not match your requirements, that's for you to figure out :P)

var li = $('<li>');

if (typeof gallery.video !== "undefined") {
    li.addClass('video');
} else {
    li.addClass('image');
}
// etc etc etc
li.appendTo("#gallery ul"); 

Also be warned that javascript does not have block scope, so your var gallery is stomping over previous declarations in the same scope.

jbabey
  • 45,965
  • 12
  • 71
  • 94
0

Here's another way to build the tags:

for (var i = 0; i < data.length; i++) {

    var gallery = data[i];
    var isShowCase = typeof gallery.showcase !== "undefined";
    var isVideo = typeof gallery.video !== "undefined";

    var $li = $('<li><a><img /></a></li>');

    if (isShowCase) {
        if (isVideo) {
            $li.addClass('video ' + gallery.showcase + (isVideo ? ' test' : '')).find('a').attr('href', gallery.link);
        }
        else {
            $li.addClass('image ' + gallery.showcase).find('a').attr('href', gallery.link).find('img').attr({ src: gallery.image, alt: gallery.title });
        }
    }
    else { 
        ....
    }

    $li.appendTo('#gallery ul');
}
Rustam
  • 746
  • 3
  • 10