Conditional jQuery format

Question

Would there be a better, cleaner way to write this conditional script? It's doing the same thing basically, except if cloneVar has the class "before", it changes which row it's grabbing (first or last) and changes .insertAfter to .insertBefore.

var cloneVar = $(this).parent().parent('.sortable');

if ($(cloneVar).hasClass('before')) {
    var cloneRow = $(cloneVar).find('.sort-group .row:first');

    $(cloneRow).clone(true).insertBefore(cloneRow)
    .addClass('add').removeClass('first')
    .find('input[type=text], textarea').val('')
    .attr('name', function(index, name) {
        return name.replace(/(\d+)/, function(fullMatch, n) {
            return Number(n) + 1;
        });
    }).parent().find('input, textarea').attr('id', function(index, id) {
        return id.replace(/(\d+)/, function(fullMatch, n) {
            return Number(n) + 1;
        });
    }).parent().find('.delete').removeClass('visible');

    return false;
} else {
    var cloneRow = $(cloneVar).find('.sort-group .row:last');

    $(cloneRow).clone(true).insertAfter(cloneRow)
    .addClass('add').removeClass('first')
    .find('input[type=text], textarea').val('')
    .attr('name', function(index, name) {
        return name.replace(/(\d+)/, function(fullMatch, n) {
            return Number(n) + 1;
        });
    }).parent().find('input, textarea').attr('id', function(index, id) {
        return id.replace(/(\d+)/, function(fullMatch, n) {
            return Number(n) + 1;
        });
    }).parent().find('.delete').removeClass('visible');

    return false;
}

You don't need to chain everything into one huge line. Break it up. — Blender, May 20 '13 at 23:38

score 1 · Accepted Answer · answered May 20 '13 at 23:59

// move this out to clean up, since you're doing the same thing in both spots
function incrementProp(index, prop) {
    return prop.replace(/(\d+)/, function (fullMatch, n) {
        return Number(n) + 1;
    });
}

var cloneRow;
if (cloneVar.hasClass('before') {
    cloneRow = $(cloneVar).find('.sort-group .row:first');
    insert = "insertBefore";
} else {
    cloneRow = $(cloneVar).find('.sort-group .row:last');
    insert = "insertAfter";
}

cloneRow.clone(true)[insert](cloneRow) // dynamically call function
    .addClass('add').removeClass('first')
    .find('input[type=text], textarea').val('')
    .attr('name', incrementProp).end()
    .find('input, textarea').attr('id', incrementProp)
    .end().find('.delete').removeClass('visible');

return false;

Actual question:

you can build the selector dynamically depending on whether cloneVar has the relevant class. Then just run the common code
Use bracket notation with a string to call insertBefore/insertAfter depending on where the new element is going

Additional pointers:

cloneVar/cloneRow are already jQuery objects, so you don't need to wrap in jQuery again
You may want to use end instead of parent - it reverts to the collection you had before calling find and you won't need to change it if you modify your DOM
You're doing a lot of chaining, but moving the replacement function out can clean it up a lot

BOOM! That works out great. Thanks Dennis and Dave for the answers! — Ryan Palmer, May 21 '13 at 00:55

score 0 · Answer 2 · answered May 20 '13 at 23:34

You can just do the if for the one part that changes:

var cloneRow = $(cloneVar).find('.sort-group .row:' + 
     /* do the check here */ (cloneVar.hasClass('before') ? 'first' : 'last'));

$(cloneRow).clone(true).insertBefore(cloneRow)
.addClass('add').removeClass('first')
.find('input[type=text], textarea').val('')
.attr('name', function(index, name) {
    return name.replace(/(\d+)/, function(fullMatch, n) {
        return Number(n) + 1;
    });
}).parent().find('input, textarea').attr('id', function(index, id) {
    return id.replace(/(\d+)/, function(fullMatch, n) {
        return Number(n) + 1;
    });
}).parent().find('.delete').removeClass('visible');

return false;

The a ? b : c syntax is just shorthand for

if (a) { b }; else { c };

`cloneVar` is already a jQuery object, so you don't need to put it in `$()` again. — Dennis, May 20 '13 at 23:37

Conditional jQuery format

2 Answers2