CODEWITHSUNDEEP
javascriptjqueryif-statement
Dizzi
Dizzi

Reputation: 747

Cleaner/simpler way of writing this jquery "if" code?

Ive written this if statement and it works fine but was just wondering if theres an easier/simpler way of doing it

if(id == 'small'){
    $('#box').append('<div class="small">' + foo + '</div>');
    $('.medium,.large').remove();
}
if(id == 'medium'){
    $('#box').append('<div class="medium">' + foo + '</div>');
    $('.small,.large').remove();
}
if(id == 'large'){
    $('#box').append('<div class="large">' + foo + '</div>');
    $('.small,.medium').remove();
}

Upvotes: 1

Views: 143

Answers (2)

David Thomas
David Thomas

Reputation: 253308

I'd suggest something like:

if (id == 'small' || id == 'medium' || id == 'large') {

    $('#box').append('<div class="' id '">' + foo + '</div>');
    $('.small, .medium, .large').not('.' + id).remove();

}

Though I suspect it could be improved, and this is, as yet, untested.

Revised example:

$('ul li').click(

function() {
    var ID = this.id;

    if (ID == 'small' || ID == 'medium' || ID == 'large') {

        $('<div />').addClass(ID).appendTo('#box');
        $('#box .small, #box .medium, #box .large')
            .not('.' + ID + ':first')
            .remove();

    }
});

JS Fiddle demo.

The above demonstration revised by the addition of + ':first' in the .not() method, in order to prevent duplicate boxes of the same class being added, as noted as a problem in the comments from @Dizzi:

This works but if you click more then once youget additional boxes.

Edited to revise a little further:

$('ul li').click(

function() {
    var ID = this.id;
    var foo = "some content, from somewhere.";

    if (ID == 'small' || ID == 'medium' || ID == 'large') {

        $('#box')
            .empty();

        $('<div />')
            .addClass(ID)
            .text(foo)
            .appendTo('#box');

    }
});

JS Fiddle demo.

The use of empty() only works if the .small, .medium and .large divs are the only contents of #box. If they are, it's slightly faster to empty the #box and then insert into it, rather than first inserting and then using a selector to remove the other contents.

If there are other contents, then the selector will (probably) still have to be used in place of empty(), though.

Edited to include the suggestion from @Raynos (in comments):

if (ID in {small:0, medium:0, large:0}) works aswell but is less readable. It's less characters though!

$('ul li').click(

function() {
    var ID = this.id;
    var foo = "some content, from somewhere.";

    if (ID in {small:0, medium:0, large:0}) {

        $('#box')
            .empty();

        $('<div />')
            .addClass(ID)
            .text(foo)
            .appendTo('#box');

    }
});

JS Fiddle demo.

References:

Upvotes: 4

James Montagne
James Montagne

Reputation: 78630

You could probably do something like:

$('#box').append('<div class="'+id+'">' + foo + '</div>');
$('.small,.medium,.large').not('.'+id).remove();

probably not the best way, but it cuts it to 2 lines. I would imagine though if I had a better understanding of the overall code, you could probably do this in a better way. It seems like this is probably called multiple times, adding and removing divs each time is not optimal.

At the very least, you should use if/else instead of separate ifs or a switch

Upvotes: 0

Related Questions