Add conditional in jquery handler or inside the function?

Question

A simple favourites system. I wrote it like this:

$(".favourite").click(function(){
  // It's active; deactivate it
  if ($(this).hasClass('active')) {
    changeFavourites('remove', -1);
    }
  // It's not active; activate it
  else {
    changeFavourites('add', 1);
    }
  });

However that was some old code and I'm refactoring it. Would this code be better?

// Handle clicking on favourites to add or remove them
// Cannot use .click() ( http://stackoverflow.com/a/12673849/938236 )
$(document).on('click', ".favourite.active", function(){
  changeFavourites('remove', -1);
  });

// :not() http://stackoverflow.com/a/520271/938236
$(document).on('click', ".favourite:not(.active)", function(){
  changeFavourites('add', 1);
  });

The main issue I see is that I'm embedding a conditional within the selector instead of making it an obvious if. I'm concerned the readability of the code will decrease and the future me will not be happy when I come back to this code. If I'm not mistaken, I think the second code could be called polymorphism.

Answer 1

While both answers are good enough, I still found them lacking a little bit (as my initial code). After much thinking and trying, something I can afford while learning, I decided to go with this code. I am using the two handlers, but also I'm adding another layer of abstraction, saveFavourites(); that will handle keyboard shortcuts and when the user gets to the element pressing tab and clicks enter when arrived to the upvote/downvote:

// Handle clicking on favourites to add or remove them
$(document).on('click', ".favourite:not(.active)", function(){
  saveFavourites('add');
  });

$(document).on('click', ".favourite.active", function(){
  saveFavourites('remove');
  });

Answer 2

This would be my preference:

$(document).on('click', ".favourite", function(){
    var active = $(this).hasClass('active');
    changeFavourites(
        active ? 'remove' : 'add', 
        active ? -1 : 1
    );
});

That's the conditional (ternary) operator -- [boolean] ? [value-if-true] : [value-if-false]

Answer 3

And here is my version:

$(document).on('click', '.favourite', function() {
    isActive = $(this).hasClass('active');
    //v1
    var method = isActive ? 'remove' : 'add';
    var flag = isActive ? -1 : 1;
    changeFavourites(method, flag);

    //v2 - bit silly
    var params = isActive ? ['remove', -1] : ['add', 1];
    changeFavourites(params[0], params[1]);
});