Search code examples
javascripthtmlcss

creating a Javascript voting system

This is my attempt at a voting system. It works as expected when there is only one. However, when I try to scale the number of voting systems beyond one it doesn't work as expected. The problem is that the voting system relies on the upVote and downVote state. So, when I increase the number of voting systems, they all rely on the same two variables, instead of each having their own upVote and downVote variables. There are two ways I think I can solve my problem.

  1. Create an array so each voting system has their own upVote and downVote variables.

  2. Use closure.

The first one I think is too cumbersome and unpractical I don't what to implement this solution. The second one I tried but couldn't get it to work.

I couldn't think of anything simpler. I'm not necessarily looking for a solution that uses closure. Any simple vanilla Javascript solution is fine.

The working code:

const up_vote_span = document.querySelector('.up-vote');
const down_vote_span = document.querySelector('.down-vote');
let count = document.querySelector('.number');

let upVote = false;
let downVote = false;

up_vote_span.addEventListener('click', function() {

  if (downVote === true) {

    up_vote_span.style.color = "#3CBC8D";
    down_vote_span.style.color = "dimgray";
    count.innerHTML = parseInt(count.innerHTML) + 2;
    upVote = true;
    downVote = false;

  } else if (upVote === false) {
    up_vote_span.style.color = "#3CBC8D";
    count.innerHTML = parseInt(count.innerHTML) + 1;
    upVote = true;
  } else if (upVote === true) {
    up_vote_span.style.color = "dimgray";
    count.innerHTML = parseInt(count.innerHTML) - 1;
    upVote = false;
  }
});

down_vote_span.addEventListener('click', function() {

  if (upVote === true) {
    up_vote_span.style.color = "dimgray";
    down_vote_span.style.color = "#3CBC8D";
    count.innerHTML = parseInt(count.innerHTML) - 2;
    downVote = true;
    upVote = false;
  } else if (downVote === false) {
    down_vote_span.style.color = "#3CBC8D";
    count.innerHTML = parseInt(count.innerHTML) - 1;
    downVote = true;
  } else if (downVote === true) {
    down_vote_span.style.color = "dimgray";
    count.innerHTML = parseInt(count.innerHTML) + 1;
    downVote = false;
  }
});
.number {
  display: inline-block;
  text-align: center;
}

.vote {
  display: flex;
  flex-direction: column;
  text-align: center;
}

.up-vote {
  color: dimgray;
  cursor: pointer;
}

.down-vote {
  cursor: pointer;
  color: dimgray;
}
<!DOCTYPE html>
<html lang="en" dir="ltr">

<head>
  <meta charset="utf-8">
  <title></title>
  <link rel="stylesheet" href="https://use.fontawesome.com/releases/v5.6.3/css/all.css" integrity="sha384-UHRtZLI+pbxtHCWp1t77Bi1L4ZtiqrqD80Kn4Z8NTSRyMA2Fd33n5dQ8lWUE00s/" crossorigin="anonymous">
</head>


  <div class="vote">
    <span class="up-vote"><i class="fas fa-angle-up"></i></span>
    <span class="number">990</span>
    <span class="down-vote"><i class="fas fa-angle-down"></i></span>
  </div>

The problematic code:

const up_vote_span = document.getElementsByClassName('up-vote');
const down_vote_span = document.getElementsByClassName('down-vote');
const count = document.getElementsByClassName('number');

let upVote = false;
let downVote = false;

for (let i = 0; i < count.length; i++) {
  up_vote_span[i].addEventListener('click', function() {

    if (downVote === true) {
      up_vote_span[i].style.color = "#3CBC8D";
      down_vote_span[i].style.color = "dimgray";
      count[i].innerHTML = parseInt(count[i].innerHTML) + 2;
      upVote = true;
      downVote = false;

    } else if (upVote === false) {
      up_vote_span[i].style.color = "#3CBC8D";
      count[i].innerHTML = parseInt(count[i].innerHTML) + 1;
      upVote = true;
    } else if (upVote === true) {
      up_vote_span[i].style.color = "dimgray";
      count[i].innerHTML = parseInt(count[i].innerHTML) - 1;
      upVote = false;
    }
  });

  down_vote_span[i].addEventListener('click', function() {

    if (upVote === true) {
      up_vote_span[i].style.color = "dimgray";
      down_vote_span[i].style.color = "#3CBC8D";
      count[i].innerHTML = parseInt(count[i].innerHTML) - 2;
      downVote = true;
      upVote = false;
    } else if (downVote === false) {
      down_vote_span[i].style.color = "#3CBC8D";
      count[i].innerHTML = parseInt(count[i].innerHTML) - 1;
      downVote = true;
    } else if (downVote === true) {
      down_vote_span[i].style.color = "dimgray";
      count[i].innerHTML = parseInt(count[i].innerHTML) + 1;
      downVote = false;
    }
  });
}
.number {
  display: inline-block;
  text-align: center;
}

.vote {
  display: flex;
  flex-direction: column;
  text-align: center;
}

.up-vote {
  color: dimgray;
  cursor: pointer;
}

.down-vote {
  cursor: pointer;
  color: dimgray;
}
<!DOCTYPE html>
<html lang="en" dir="ltr">

<head>
  <meta charset="utf-8">
  <title></title>
  <link rel="stylesheet" href="https://use.fontawesome.com/releases/v5.6.3/css/all.css" integrity="sha384-UHRtZLI+pbxtHCWp1t77Bi1L4ZtiqrqD80Kn4Z8NTSRyMA2Fd33n5dQ8lWUE00s/" crossorigin="anonymous">
</head>

<body>

  <div class="vote">
    <span class="up-vote"><i class="fas fa-angle-up"></i></span>
    <span class="number">990</span>
    <span class="down-vote"><i class="fas fa-angle-down"></i></span>
  </div>
  <div class="vote">
    <span class="up-vote"><i class="fas fa-angle-up"></i></span>
    <span class="number">990</span>
    <span class="down-vote"><i class="fas fa-angle-down"></i></span>
  </div>
  <div class="vote">
    <span class="up-vote"><i class="fas fa-angle-up"></i></span>
    <span class="number">990</span>
    <span class="down-vote"><i class="fas fa-angle-down"></i></span>
  </div>
  <div class="vote">
    <span class="up-vote"><i class="fas fa-angle-up"></i></span>
    <span class="number">990</span>
    <span class="down-vote"><i class="fas fa-angle-down"></i></span>
  </div>

</body>

</html>

Solution

  • I think you will find this easier to control and think about if you go with the array method you mentioned as option #1 and then make a single function for upvoting and a single function for downvoting instead of creating separate functions for every single up and down arrow like you are doing now.

    You have four groups right now, so inside your for loop we can initialize an array like this:

    const votes = [
  0: { up: false, down: false },
  1: { up: false, down: false },
  2: { up: false, down: false },
  3: { up: false, down: false },
];

    Then you can just call your up- and down-voting functions and check the value of the object in that array that corresponds to the group number of the arrow you clicked.

    The other thing that I think helps readability is separating out changing the vote total from the color changes.

    Your vote logic includes three distinct possibilities, 1.) arrow was already checked, 2.) opposite arrow was checked, and 3.) neither arrow was checked. But your color-changing logic and the logic for changing the checked value of each arrow really only has two possibilities: either the arrow was checked before or it wasn't.

    So I went ahead and made that change in the below snippet as well.

    Hope this helps.

    const up_vote_spans = document.getElementsByClassName('up-vote');
const down_vote_spans = document.getElementsByClassName('down-vote');
const count = document.getElementsByClassName('number');

let votes = [];

for (let i = 0; i < count.length; i += 1) {
  const thisUpVoteSpan = up_vote_spans[i];
  const thisDownVoteSpan = down_vote_spans[i];
  votes[i] = { up: false, down: false };

  thisUpVoteSpan.addEventListener('click', handleUpvote.bind(null, i), false);
  thisDownVoteSpan.addEventListener('click', handleDownvote.bind(null, i), false);
}

function handleUpvote(i) {
  const currentVote = votes[i];
  const matchingUpSpan = up_vote_spans[i];
  const matchingDownSpan = down_vote_spans[i];
  const matchingCount = count[i];
  const currentCount = parseInt(matchingCount.innerHTML);

  if (currentVote.down) {
    matchingCount.innerHTML = currentCount + 2;
  } else if (currentVote.up === false) {
    matchingCount.innerHTML = currentCount + 1;
  } else {
    matchingCount.innerHTML = currentCount - 1;
  }
  if (!currentVote.up) {
    matchingUpSpan.style.color = "#3CBC8D";
    matchingDownSpan.style.color = 'dimgray';
    currentVote.up = true;
    currentVote.down = false;
  } else {
    matchingUpSpan.style.color = 'dimgray';
    currentVote.up = false;
  }
}

function handleDownvote(i) {
  const currentVote = votes[i];
  const matchingUpSpan = up_vote_spans[i];
  const matchingDownSpan = down_vote_spans[i];
  const matchingCount = count[i];
  const currentCount = parseInt(matchingCount.innerHTML);

  if (currentVote.up) {
    matchingCount.innerHTML = currentCount - 2;
  } else if (currentVote.down === false) {
    matchingCount.innerHTML = currentCount - 1;
  } else {
    matchingCount.innerHTML = currentCount + 1;
  }
  if (!currentVote.down) {
    matchingDownSpan.style.color = "#3CBC8D";
    matchingUpSpan.style.color = 'dimgray';
    currentVote.down = true;
    currentVote.up = false;
  } else {
    matchingDownSpan.style.color = 'dimgray';
    currentVote.down = false;
  }
}
    .number {
  display: inline-block;
  text-align: center;
}

.vote {
  display: flex;
  flex-direction: column;
  text-align: center;
}

.up-vote {
  color: dimgray;
  cursor: pointer;
}

.down-vote {
  cursor: pointer;
  color: dimgray;
}
    <!DOCTYPE html>
<html lang="en" dir="ltr">

<head>
  <meta charset="utf-8">
  <title></title>
  <link rel="stylesheet" href="https://use.fontawesome.com/releases/v5.6.3/css/all.css" integrity="sha384-UHRtZLI+pbxtHCWp1t77Bi1L4ZtiqrqD80Kn4Z8NTSRyMA2Fd33n5dQ8lWUE00s/" crossorigin="anonymous">
</head>

<body>

  <div class="vote">
    <span class="up-vote"><i class="fas fa-angle-up"></i></span>
    <span class="number">990</span>
    <span class="down-vote"><i class="fas fa-angle-down"></i></span>
  </div>
  <div class="vote">
    <span class="up-vote"><i class="fas fa-angle-up"></i></span>
    <span class="number">990</span>
    <span class="down-vote"><i class="fas fa-angle-down"></i></span>
  </div>
  <div class="vote">
    <span class="up-vote"><i class="fas fa-angle-up"></i></span>
    <span class="number">990</span>
    <span class="down-vote"><i class="fas fa-angle-down"></i></span>
  </div>
  <div class="vote">
    <span class="up-vote"><i class="fas fa-angle-up"></i></span>
    <span class="number">990</span>
    <span class="down-vote"><i class="fas fa-angle-down"></i></span>
  </div>

</body>

</html>