HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Simplifying up/down vote code

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codevotedownsimplifying

Problem

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()) + parseI

Solution

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); });

Code Snippets

parseInt(1)
$(this).parent().children('.post-votes')
$(this).css('background-color')
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); });

Context

StackExchange Code Review Q#4312, answer score: 6

Revisions (0)

No revisions yet.