Is there a way to refactor this javascript/jquery? [closed]

This question is unlikely to help any future visitors; it is only relevant to a small geographic area, a specific moment in time, or an extraordinarily narrow situation that is not generally applicable to the worldwide audience of the internet. For help making this question more broadly applicable, visit the help center. Closed 11 years a开发者_如何转开发go.

switch (options.effect) {

case 'h-blinds-fadein':
    $('.child').each(function(i) {
        $(this).stop().css({
            opacity: 0
        }).delay(100 * i).animate({
            'opacity': 1
        }, {
            duration: options.speed,
            complete: (i !== r * c - 1) ||
            function() {
                $(this).parent().replaceWith(prev);
                options.cp.bind('click', {
                    effect: options.effect
                }, options.ch);
            }
        });
    });

    break;

case 'h-blinds-fadein-reverse':
    $('.child').each(function(i) {
        $(this).stop().css({
            opacity: 0
        }).delay(100 * (r * c - i)).animate({
            'opacity': 1
        }, {
            duration: options.speed,
            complete: (i !== 0) ||
            function() {
                $(this).parent().replaceWith(prev);
                options.cp.bind('click', {
                    effect: options.effect
                }, options.ch);
            }
        });
    });

    break;

    ....more cases
}

I have alot of similiar other cases. One way i could think of is to write functions ? i'm not sure i'm still fairly new to the language

im sorry, i is the index of the each() function which is the size of $('.child'), and r and c are just the 'rows' and 'columns' of the grid which contains '.child'. r and c can be any number, e.g. r=5 c=5

---

Rather then using a switch, store the case specific data in a hash.

Then run the main block of code and extract anything effect type specific from the hash.

function doit(e) {
    var hash = {
        'h-blinds-fadein': {
            delay: function(i) { return i; },
            complete: function(i) { return (i !== r * c - 1); }
        },
        'h-blinds-fadein-reverse': {
            delay: function(i) { return (r * c - i); },
            complete: function(i) { return i !== 0; }
        }
    }

    $('.child').each(function(i) {
        $(this).stop().css({
            opacity: 0
        }).delay(100 * hash[e].delay(i)).animate({
            'opacity': 1
        }, {
            duration: options.speed,
            complete: hash[e].complete(i) ||
            function() {
                $(this).parent().replaceWith(prev);
                options.cp.bind('click', {
                    effect: options.effect
                }, options.ch);
            }
        });
    });
}

doit(options.effect);

---

Without knowing what i, r and c are, it's very hard to refactor. This can be condensed to a function passing those numbers as variables - as ultimately all the other parts of the code are the same.

---

I think using a function in each of the case statements would be the first thing to do.

case 'h-blinds-fadein':
    hBlindsFadeIn(options);    
    break;

...

function hBlindsFadeIn(options){
       $('.child').each(function(i) {
            $(this).stop().css({
                opacity: 0
            }).delay(100 * i).animate({
                'opacity': 1
            }, {
                duration: options.speed,
                complete: (i !== r * c - 1) ||
                function() {
                    $(this).parent().replaceWith(prev);
                    options.cp.bind('click', {
                        effect: options.effect
                    }, options.ch);
                }
            });
        });
}

Obviously your function would need to be outside of the switch.

---

All I could suggest would be to refactor the actual effects into separate methods, this would make your switch statement a bit more readable, i.e.:

switch (options.effect) {

case 'h-blinds-fadein':
    h-blinds-fadein();
    break;

case 'h-blinds-fadein-reverse':
    h-blinds-fadein-reverse();
    break;

    ....more cases
}

function h-blinds-fadein() {
    $('.child').each(function(i) {
        $(this).stop().css({
            opacity: 0
        }).delay(100 * i).animate({
            'opacity': 1
        }, {
            duration: options.speed,
            complete: (i !== r * c - 1) ||
            function() {
                $(this).parent().replaceWith(prev);
                options.cp.bind('click', {
                    effect: options.effect
                }, options.ch);
            }
        });
    });
}

function h-blinds-fadein-reverse() {
    $('.child').each(function(i) {
        $(this).stop().css({
            opacity: 0
        }).delay(100 * (r * c - i)).animate({
            'opacity': 1
        }, {
            duration: options.speed,
            complete: (i !== 0) ||
            function() {
                $(this).parent().replaceWith(prev);
                options.cp.bind('click', {
                    effect: options.effect
                }, options.ch);
            }
        });
    });
}

Alternatively you could try the eval function with the refactored functions thus:

eval(options.effect+"();");

---- EDIT ----

Love Raynos refactoring, this could also be incorporated.