Javascript "for" loop issue when using ajax, the loop has access even when condition is false

Question

Working with laravel, the system we were developing was crashing on this view on the "onready" event. After looking at the code, we found the problem was inside the for loop.

$(document).ready(function() {
var url = "{{ route('routeToGetAuctions') }}";
$.ajax({
  url : url,
  type : "GET"
})
.done(function(data) {
  for (var j = 0; j < data.auctions.length; j++) {
    ...
    $('#object').downCount(data.auctions[j].auction, {
      startDate: data.auctions[j].startDate,
      endDate: data.auctions[j].endDate,
      offset: data.auctions[j].offset
    }, function() {
        //Class management to show the auctions
        finishedAuctions.push(data.auctions[j].auction);
        $('#countdown-bg-'+data.auctions[j].auction).removeClass('bg-primary');
        $('#countdown-bg-'+data.auctions[j].auction).addClass('bg-secondary');
        $('#progress-'+data.auctions[j].auction).removeClass('bg-primary');
        $('#progress-'+data.auctions[j].auction).addClass('bg-secondary');
    });
  }
});

This works perfect for what we needed... But, assuming there exist 3 available auctions, data.auctions.length will have a value of 3, and doing a console.log('value of j: ' + j) to debug the for loop, it somehow prints:

value of j: 0
value of j: 1
value of j: 2
value of j: 3

and then crashes as it tries to reach index 3 of an array with size 3 (0,1,2).

A pseudo fix we made is a try catch code block, as this problem persisted no matter how many items there existed in the array and it always reached the last index + 1:

$(document).ready(function() {
    var url = "{{ route('routeToGetAuctions') }}";
    $.ajax({
      url : url,
      type : "GET"
    })
    .done(function(data) {
      for (var j = 0; j < data.auctions.length; j++) {
        ...
        $('#object').downCount(data.auctions[j].auction, {
          startDate: data.auctions[j].startDate,
          endDate: data.auctions[j].endDate,
          offset: data.auctions[j].offset
        }, function() {// Try Catch to fix unknown problem with loop
          try {
            finishedAuctions.push(data.auctions[j].auction);
            $('#countdown-bg-'+data.auctions[j].auction).removeClass('bg-primary');
            $('#countdown-bg-'+data.auctions[j].auction).addClass('bg-secondary');
            $('#progress-'+data.auctions[j].auction).removeClass('bg-primary');
            $('#progress-'+data.auctions[j].auction).addClass('bg-secondary');
          } catch (e) {
            //here the index exception is prevented and the view won't crash.
          }
        });
      }
    });

Simple and stupid fix we made, but we haven't found out WHY this is happening, how is the for loop, assuming data.auctions.length = 3, printing 0,1,2,3?

The code inside the callback (where you've put the `try...catch` will run *after* the loop has finished and would be referencing the `j` at that final state. So, it will be `j = data.auctions.length` in all instances. — VLAZ, Dec 18 '19 at 16:50

Jeffrey Devloo · Accepted Answer · 2019-12-18T17:12:04.927

The downCount callback is executed once the countdown is finished. Meaning that this isn't executed right away.

Because of this, your loop keeps incrementing 'j' which means that once the callback is executed, 'j' will be at the maximum value.

Here is a simple demonstration of what you are experiencing. It will log '5' multiple times, as opposed to 0,1,2,3,4. The reason why it is 5 is because the i will increment first and then the condition is checked! This is exactly why your code is crashing. Since j increments up to one more than the array length, to check the condition!

for (var i = 0; i < 5; i++) {
  setTimeout(function() {
    console.log(i)
  }, 100)
}

The easiest solution is to use let as opposed to var as they are scope bound.

for (let i = 0; i < 5; i++) {
  setTimeout(function() {
    console.log(i)
  }, 100)
}

If you don't have access to let, you can use a closure

for (var i = 0; i < 5; i++) {
  (function(val) {
    setTimeout(function() {
    console.log(val)
  }, 100)
  })(i)
}

@Vlaz was correct with his scoping article. It is a great read to enlighten you even further!

Please use the built-in StackSnippets feature to post runnable examples. This makes the post self-contained and a lot easier to follow, without requiring off-site resources. It also prevents the post from losing value if the off-site links happen to stop working. You can create a runnable snippet by using the `[<>]` button. [Check here for more info](https://meta.stackoverflow.com/questions/358992/ive-been-told-to-create-a-runnable-example-with-stack-snippets-how-do-i-do). — VLAZ, Dec 18 '19 at 17:06
Oh, thanks for the update! I did not know! I'll update my answer shortly. — Jeffrey Devloo, Dec 18 '19 at 17:07

Javascript "for" loop issue when using ajax, the loop has access even when condition is false

1 Answers1

Linked