State machine for upvoting/downvoting

Question

I have a list of resources which can be voted on. There are 3 voting states: upvoted, downvoted and no vote.

In a fashion identical to Stack Exchange voting, it follows these rules:

1. If you upvote a post you've already upvoted, it will set it to 'no vote'
2. If you upvote a post set to 'no vote' it will set it to upvote
3. if you downvote a post you've upvoted, it will set it to 'downvote' (I bolded this because in another post everyone gave me grief that this way is 'stupid', yet every voting site I have seen uses this style... Reddit, Digg. Stack Overflow, etc)

The exact same rules (except inverse) hold true for downvote.

Here's how I call my sendVote() function, when you click on upvote or downvote:

  // Voting
  $('.res-list').delegate('.vote .up', 'click', function() {
    flashColor($(this),'#fffc00');
    sendVote($(this).parent(),'upvoted');
    return false;
  });

  $('.res-list').delegate('.vote .down', 'click', function() {
    flashColor($(this),'#fffc00');
    sendVote($(this).parent(),'downvoted');
    return false;
  });

I believe that above code is fine, but his code below is what I feel is awful and could really be refactored, but I don't know how. I'm kind of new to JavaScript and jQuery.

function sendVote(object, direction) {

  var votesContainer = $(object).find('.m p');
  var votes = parseInt($(votesContainer).html());

  // UPVOTED
  if (direction == 'upvoted') {

    if ($(object).hasClass('downvoted')) {
      // Overwrite Downvote
      $(votesContainer).html(votes + 2);
      $(object).removeClass('downvoted');
      $(object).addClass(direction);
    } else if ($(object).hasClass(direction)) {
      // Undo Upvote 
      $(votesContainer).html(votes - 1);
      $(object).removeClass(direction);
    } else {
      // Do Upvote
      $(votesContainer).html(votes + 1);
      $(object).addClass(direction);
    };

    // Ajax function here to /votes/upvote?id=x

  // DOWNVOTED
  } else {

    if ($(object).hasClass('upvoted')) {
      // Overwrite Upvote
      $(votesContainer).html(votes - 2);
      $(object).removeClass('upvoted');
      $(object).addClass(direction);
    } else if ($(object).hasClass(direction)) {
      // Undo Downvote
      $(votesContainer).html(votes + 1);
      $(object).removeClass(direction);
    } else {
      // Do Downvote
      $(votesContainer).html(votes - 1);
      $(object).addClass(direction);
    };

    // Ajax function here to /votes/downvote?id=x

  }
}

Answer 1

Both portions of code could be simplified if you use numbers, instead of words, to represent upvotes and downvotes. So I would change your calls to sendVote to look like this:

sendVote(this.parentNode, 1);
sendVote(this.parentNode, -1);

When modifying elements with javascript, I suggest using element.style.x (or element.style.cssText for multiple attributes) instead of classes, where possible. This is more efficient (browser doesn't have to re-test css selectors for the rest of the page), simpler (don't have to polyfill classList), and it means putting javascript-only css in js files, which means they aren't loaded in browsers that don't support javascript and therefore don't want them anyway. But the main reason is that is allows you to shorten your code to this:

var voteCSS = {
    '1': 'background:blue;',
    '0': 'background:grey;',
    '-1': 'background:red;'
};

function sendVote(element, newVote) {
    var countEl = element.querySelector('.m p'),
        prevVote = +element.dataset.prevVote || 0;

    if (newVote == prevVote)
        newVote = 0;
    countEl.innerHTML = +countEl.innerHTML - prevVote + newVote;
    element.dataset.prevVote = newVote;
    element.style.cssText = voteCSS[newVote];
    // ajax here to /votes/submit?id=x&direction=`newVote`
}

Alternatively, if you want to keep all your CSS in one place, you can make sure that "upvoted" or "downvoted" is the only class the element has, then just use .className. You could still apply default styling by selecting it from above, something like this: .votecontainer div {}.

ps: notice that I didn't use jQuery even once. Try to solve your problems without jQuery before you let yourself use it - a lot of the time you'll find that your solutions are simpler and more efficient without it.