CODEWITHSUNDEEP
javascriptjquery
dee cheok
dee cheok

Reputation: 257

increase and decrease letter

var letterbox = ["A", "B", "C", "D", "E", "F", "G", "H", "I",];

var counter = 0;

  $(".plusbtn").click(function(){

    if(counter >= letterbox.length - 1){
        counter = -1;
    }

    counter++;

    $(".ux--wrapper").empty();
    $(".ux--wrapper").append("<p>"+letterbox[counter]+"</p>");
  });

  $(".minbtn").click(function(){
      if(counter <= 0){
        counter = letterbox.length;
        }

    counter--;
    $(".ux--wrapper").empty();
    $(".ux--wrapper").append("<p>"+letterbox[counter]+"</p>");
  });

my intention the letter start at A and when you press plus it keep increase to B , c , D and so on. when you press min it decrease , but because the number start at 0 , so i have some problem at plusing i have to use -1 to make it right , but i feel i am doing wrong.

i am i doing the right way here ? can you make it more low risky ? i feel like my code is risky

Demo here

Upvotes: 0

Views: 91

Answers (3)

yolenoyer
yolenoyer

Reputation: 9445

Your code is error free, but it can be simplified in multiple ways. Here is one of them:

var letterbox = ["A", "B", "C", "D", "E", "F", "G", "H", "I",];

var counter = 0;

function update_counter(inc)
{
    counter = (counter + inc + letterbox.length) % letterbox.length;
    $(".ux--wrapper").empty().append("<p>"+letterbox[counter]+"</p>");
}

$(".plusbtn").click(function() { update_counter(1) });
$(".minbtn").click(function() { update_counter(-1) });

By creating a function that handles the common part to execute between the two buttons, you greatly reduces the code size.

Note the double use of letterbox.length inside the modulo operation: it lets you handle negative increments (considering that this increment is -1, or at least greater than -letterbox.length)

Upvotes: 1

gyre
gyre

Reputation: 16779

You can use the modulo (similar to remainder) operator, %, and data-increment attributes on each button to reduce the number of event listeners necessary and make your code more readable:

var letters = 'ABCDEFGHI'.split('')
var counter = 0

var output = $('.ux--wrapper p')[0]

$('.plusbtn, .minbtn').click(function() {

  counter += +$(this).data('increment')

  if (counter < 0) counter += letters.length
  else counter %= letters.length

  output.textContent = letters[counter]
})
.minbtn,
.plusbtn {
  float: left;
  cursor: pointer;
}

.ux--wrapper {
  width: 100px;
  height: 50px;
  background-color: #000;
  color: #fff;
  text-align: center;
  float: left;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<button class="minbtn" data-increment="-1">-</button>
<div class="ux--wrapper">
  <p>A</p>
</div>
<button class="plusbtn" data-increment="+1">+</button>

Upvotes: 0

bcr666
bcr666

Reputation: 2197

var letterbox = ["A", "B", "C", "D", "E", "F", "G", "H", "I",];

var counter = 0;

$(".plusbtn").click(function(){
    if(counter < letterbox.length){
        counter++;
    } else {
        counter = 0;
    }
    displayCounter();
});

function displayCounter() {
    $(".ux--wrapper").empty();
    $(".ux--wrapper").append("<p>"+letterbox[counter]+"</p>");
});

$(".minbtn").click(function(){
    if(counter > 0){
        counter--;
    } else {
        counter = letterbox.length;
    }
    displayCounter();
});

Upvotes: 0

Related Questions