Simplifying up/down vote code

Question

I've got this working JavaScript (example is [here][1]), which should work the same as on the Stack Exchange network.

How can I simplify it a bit?

function yellow() {
 return 'rgb(255, 255, 0)';
}
$(function() {
 $(".post-upvote").click(function() {
 // ajax(url + "upvote/" + $(this).attr('data-postid'), false, false);
 if ($(this).parent().children('.post-downvote').css('background-color') == yellow()) { // user upvoted so let's delete upvote
 $(this).parent().children('.post-votes').text(parseInt($(this).parent().children('.post-votes').text()) + parseInt(1));
 }
 $(this).parent().children('.post-downvote').css('background-color', 'white');
 if ($(this).css('background-color') == yellow()) { // if it's yellow, user is canceling his downvote
 $(this).css('background-color', 'white');
 $(this).parent().children('.post-votes').text(parseInt($(this).parent().children('.post-votes').text()) - parseInt(1));
 }
 else {
 $(this).css('background-color', 'yellow');
 $(this).parent().children('.post-votes').text(parseInt($(this).parent().children('.post-votes').text()) + parseInt(1));
 }
 });
 $(".post-downvote").click(function() {
 // ajax(url + "downvote/" + $(this).attr('data-postid'), false, false);
 if ($(this).parent().children('.post-upvote').css('background-color') == yellow()) { // user upvoted so let's delete upvote
 $(this).parent().children('.post-votes').text(parseInt($(this).parent().children('.post-votes').text()) - parseInt(1));
 }
 $(this).parent().children('.post-upvote').css('background-color', 'white');
 if ($(this).css('background-color') == yellow()) { // if it's yellow, user is canceling his downvote
 $(this).css('background-color', 'white');
 $(this).parent().children('.post-votes').text(parseInt($(this).parent().children('.post-votes').text()) + parseInt(1));
 }
 else {
 $(this).css('background-color', 'yellow');
 $(this).parent().children('.post-votes').text(parseInt($(this).parent().children('.post-votes').text()) - parseInt(1));
 }
 });
});

Answer 1

Get rid of antipatterns like

parseInt(1)

Just say 1 instead.

Factor out common code like

$(this).parent().children('.post-votes')

and

$(this).css('background-color')

into variables.

Combine the two handlers thus

function vote(isUpvote) {
 var control = $(this);
 var otherControl = control.parent().children(
 isUpvote ? ".post-downvote" : ".post-upvote");
 var postVotes = control.parent().children(".post-votes");
 var ajaxHandler = isUpvote ? "upvote/" : "downvote/";
 // ajax(url + ajaxHandler + control.attr('data-postid'), false, false);
 // user voted so let's delete the other control
 if (otherControl.css("background-color") == yellow()) {
 postVotes.text(+(postVotes.text()) + 1);
 }
 control.parent().children(otherId).css("background-color", "white");
 // if it's yellow, user is cancelling their vote
 if (control.css("background-color") == yellow()) {
 control.css("background-color", "white");
 postVotes.text(+(postVotes.text()) - 1);
 } else {
 control.css("background-color", "yellow");
 postVotes.text(+(postVotes.text()) + 1);
 }
}
$(".post-upvote" ).click(function() { vote.call(this, true); });
$(".post-downvote").click(function() { vote.call(this, false); });

• genesis: I wasn't able to do something + 1 because it made 12 from 1
• Mike Samuel: @genesis, That happens when something is not a number because then + does string concatenation instead of addition. parseInt(something, 10) + 1 is fine as is (+something) + 1 because they both cause the something to be coerced to a number. The parseInt(1) does not since 1 is already a number.
• genesis: ok so I can "convert" string to number with (+var) + 1 ?
• Mike Samuel: @genesis, Yes, assuming "var" is the name of a variable instead of the keyword. Try running +"42" + 1 === 43 in the squarefree shell.
• genesis: and about your last snippet, I am not 100% how should I include it effectively. Could you add some more please?