If statement doesn't seem to work in my jQuery code, althought it should

Question

I have a function that does that a div-box called Karusell is being moved x px to the side by clicking on a different div-box called bokse1. The change is also animated. I also have a function that does that every time I click on bokse1 "1" is being added to "nextClicked". I tried to make it so that when nextClicked = 2 the first function will stop working by doing if (nextClicked != 2), but it doesn't work for some strange reason.

Here is a fiddle: http://jsfiddle.net/vbLjcpx1/7/

Code:

currentNextClicked = 0;

nextClicked = 0;

$(document).ready(function() {

  if (nextClicked != 2) {

    $("#bokse1").click(function() {
      $("#Karusell").animate({
        marginLeft: ["+=10px", "linear"],
      }, 400, function() {});
    });

    $('#bokse1').live('click', function() {
      currentNextClicked = nextClicked;
      nextClicked = currentNextClicked + 1;
      console.log(nextClicked)
    });

  }
});

#bokse1 {
  width: 20px;
  height: 20px;
  background-color: blue;
}

#bokse2 {
  width: 20px;
  height: 20px;
  background-color: red;
}

#Karusell {
  width: 30px;
  height: 30px;
  background-color: indigo;
  display: absolute;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/1.6.4/jquery.min.js"></script>
<div id="bokse1"></div>
<div id="bokse2"></div>

<div id="Karusell">

once you bind an event to an element, in order to reset it you need to `.unbind()` it — GrafiCode, May 06 '19 at 09:56
@Krøsh your if condition should be inside handler and not outside — Rajesh, May 06 '19 at 10:01
@Rajesh wow that was an easy fix, althought I dont understand why I cant have one if statement for every function, because now I need to put the if statement under both of the function. All in all, thank you! — Krøsh, May 06 '19 at 10:06
as you seems to be beginnner, a little thing that can help you is the debugger view, you can do: debugger; on any line of your js to stop the process of your code and to do it step by step. — Aurele Collinet, May 06 '19 at 14:50

score 0 · Answer 1 · answered May 06 '19 at 10:03

0

The listenner on the bokse1 wont stop with your if,

to prevent the code to happen place your if after its clicked like:

$("#bokse1").click(function() {
 if (nextClicked != 2) {
  $("#Karusell").animate({
    marginLeft: ["+=10px", "linear"],
  }, 400, function() {});
 }
});

Because when you click JS goes directly inside your listener, the beginning of the code is called only once when you load the page

answered May 06 '19 at 10:03

Aurele Collinet

138
1
16

If I may ask, why there is a need for callback function in `animate`? – Yash Karanke May 06 '19 at 10:05
i think their is no need to do it, as its his code i didn't change it. Maybe he plans to do stuff in there when the animation is done – Aurele Collinet May 06 '19 at 10:08
@YashKaranke duude, I know I need to have a callback because without it the animation won't happen, but I don't know why it won't happen without it. – Krøsh May 06 '19 at 10:50

score 0 · Answer 2 · edited Jun 20 '20 at 09:12

As per your code, your if condition is in $(document).ready. So when this callback will be called, then your if will be evaluated. However, it will not be called for every click.

On click only following code will be evaluated which does not have any condition.

$("#bokse1").click(function() {
  $("#Karusell").animate({
    marginLeft: ["+=10px", "linear"],
  }, 400, function() {});
});

$('#bokse1').live('click', function() {
  currentNextClicked = nextClicked;
  nextClicked = currentNextClicked + 1;
  console.log(nextClicked)
});

Also, in my understanding, you do not need 2 handlers. It can be handled in 1 as well

$("#bokse1").click(function() {
  if (currentNextClicked++ < 2) {
    $("#Karusell").animate({
      marginLeft: ["+=10px", "linear"],
    }, 400, function() {});
  }
  console.log(currentNextClicked)
});

Also per the code shared, following code doesn't makes sense:

$('#bokse1').live('click', function() {
  currentNextClicked = nextClicked;
  nextClicked = currentNextClicked + 1;
  console.log(nextClicked)
});

You have numeric values stored in 2 variables. If I have to guess, currentNextClicked refers to current iteration and nextClicked points to next. But this can be achieved with one variable as well, as nextClicked will always be currentNextClicked + 1. Also if you notice, if in above code is < 2. That is because, != 2 will be valid if nextClicked becomes more than 2. Using less than operator would be a better option if you wish to restrict it to 2 values only.

Suggestions:

$().live has been deprecated. So you will not be able to upgrade easily. So as a suggestion, try to avoid it.
Declaring a variable without var|const|let makes it global variable. Its a bad practice to bleed variables to global scope. So, always use a keyword to declare variable.
Declaring a variable outside any function will also make it global. So try to even avoid that. Ideally, a variable should have restricted and very specific scope. That means, declaring a variable only in area it will be used.
!= will perform a loose check. That means 2 != '2' will return false but it should return true. Try using !== to enforce that passed variable is a number only.

Sample code:

$(document).ready(function() {
  var currentNextClicked = 0;
  $("#bokse1").on('click', function() {
    if (currentNextClicked++ < 2) {
      $("#Karusell").animate({
        marginLeft: ["+=10px", "linear"],
      }, 400, function() {});
    }
    console.log(currentNextClicked)
  });
});

#bokse1 {
  width: 20px;
  height: 20px;
  background-color: blue;
}

#bokse2 {
  width: 20px;
  height: 20px;
  background-color: red;
}

#Karusell {
  width: 30px;
  height: 30px;
  background-color: indigo;
  display: absolute;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/1.6.4/jquery.min.js"></script>
<div id="bokse1"></div>
<div id="bokse2"></div>

<div id="Karusell">

If statement doesn't seem to work in my jQuery code, althought it should

2 Answers2

Suggestions:

Reference: