1
\$\begingroup\$

I'm finally "done" with my small prototype for an element inside my website. I'm just a learner in the jQuery code and that reflects in my code. It's bloated, a lot of functions do the same thing, but for a red or green button which results in this kind of code.

$('.submenu').on('click','.rood',function() {
    $(this).closest('.container_vragen').fadeOut(400, function() {
        $(this).closest('.container_vragen').css({overflow : 'hidden', color : 'red'});
        $(this).closest('.container_vragen').appendTo("#niet_geregeld");
        $(this).closest('.container_vragen').fadeIn(400);
    });
});

$('.submenu').on('click','.groen',function() {
    $(this).closest('.container_vragen').fadeOut(400, function() {
        $(this).closest('.container_vragen').css({overflow : 'hidden', color : 'green'});
        $(this).closest('.container_vragen').appendTo("#geregeld");
        $(this).closest('.container_vragen').fadeIn(400);
    });
});

As you can see most of the functions are the same, only the color and the .appedTo id changes. What's the best way to merge this code? I'm thinking there needs to be an if statement which filters what button is clicked. But I can't figure it out on my own.

\$\endgroup\$
1
\$\begingroup\$

You should think about chaining, to avoid repeating some class names :

function doAnimation($element, $destElement, animationColor) {
    var $container_vragen = $element.closest('.container_vragen');
    $container_vragen.fadeOut(400, function() {
        $container_vragen.css({
                overflow : 'hidden',
                color : animationColor
            })
            .appendTo($destElement)
            .fadeIn(400);
    });
}

var $niet_geregeld = $('#niet_geregeld');
var $geregeld = $('#geregeld');

$('.submenu')
.on('click','.rood',function() {
    doAnimation($(this), $niet_geregeld,'red');
})
.on('click','.groen',function() {
    doAnimation($(this), $geregeld,'green');
});

I think preselecting #geregeld and #niet_geregeld is better for performances.

You should rename the doAnimation function to its real purpose, like animateSorting or animateForThatPurpose

\$\endgroup\$
1
\$\begingroup\$

Store your options separately, then loop over them:

var $submenu = $('.submenu');
var map = {
    'rood': {
        color: 'red',
        appendTo: 'niet_geregeld'
    },
    'groen': {
        color: 'green',
        appendTo: 'geregeld'
    }
};

$.each(map, function (elClass, options) {
    $submenu.on('click', '.' + elClass, function () {

        var $container = $(this).closest('.container_vragen');

        $container.fadeOut(400, function() {
            $container
                .css({overflow : 'hidden', color : options.color})
                .appendTo("#" + options.appendTo)
                .fadeIn(400);
        });

    });
});
\$\endgroup\$
1
\$\begingroup\$

Make a function.

function doStuff(obj, c) {
  obj.css({overflow: 'hidden', color: c}).fadeIn(400);
}

Then call it

$('#submenu').click(function () {
  doStuff($('#geregeld'), 'green');
});

(simplified example)

Feel free to mess around: http://jsfiddle.net/rQFHt/

\$\endgroup\$
  • \$\begingroup\$ I understand what you're trying to do, but it's not working. If I use this code the click action on .groen is not calling the doStuff function. \$\endgroup\$ – Peter Boomsma Jan 10 '13 at 18:50
  • \$\begingroup\$ @PeterBoomsma : updated :) \$\endgroup\$ – user1882585 Jan 10 '13 at 19:50
1
\$\begingroup\$

You can combine the two like this

$('.submenu').on('click', '.groen,.rood', function () {
  var $el = $(this);
  var red = $el.is('.rood'); // is this element .rood
  $el.closest('.container_vragen').css({
    overflow: 'hidden',
    color: red ? 'red' : 'green' // if it's .rood make it red.. else make green
  })
    .appendTo(red ? "#niet_geregeld" : "#geregeld") // if .rood appendto #niet_geregeld else append to #geregeld
    .fadeIn(400)
});

and use chaining so you don't have to keep querying the dom for the element.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.