0

My code is pretty simple, but for some reason I am getting different result depending on which method I use (chartIsVisible or chartIsVisible2). What is making the difference?

function chartIsVisible(id) {
    for (var i = 0; i < charts.length; i++) {
        if (charts[i].Id == id)
            return true;
    }
    return false;
}

function chartIsVisible2(id) {
    charts.forEach(function (chart) {
        if (chart.Id == parseInt(id))
            return true;
    });
    return false;
}

function insertMissingQueues(responses) {
    responses.forEach(function (response) {
        if (chartIsVisible(response.Id)) {
            console.log("IT IS VISIBLE");
        } else {
            console.log("ITS NOT VISIBLE"); 
        }

        if (chartIsVisible2(response.Id)) {
            console.log("IT IS VISIBLE");
        } else {
            console.log("ITS NOT VISIBLE");
        }
    });

    addChart(responses.Id);
}

insertMissingQueues(response);
output: ITS NOT VISIBLE
output: ITS NOT VISIBLE

insertMissingQueues(response);
output: IT IS VISIBLE
output: ITS NOT VISIBLE // I expected different result there..
dafie
  • 951
  • 7
  • 25
  • 4
    A `return` statement inside a `.forEach()` callback only returns from the callback, not the containing function. – Pointy Sep 12 '18 at 14:05
  • 3
    Possible duplicate of [What does \`return\` keyword mean inside \`forEach\` function?](https://stackoverflow.com/questions/34653612/what-does-return-keyword-mean-inside-foreach-function) – Luca Kiebel Sep 12 '18 at 14:06
  • Not really an answer but you don't need to do `if (chart.Id == parseInt(id)) return true;`. You can just do `return chart.Id == parseInt(id);` – Michael Curry Sep 12 '18 at 14:07
  • @MichaelCurry but that is not semantically the same `return 1 == 1` and `return 1 ==2` both *return* immediately, when the intention thus ending the `for` loop early. The intention is to only do that if you find a match, otherwise the code is equivalent to `return chart[0].Id == parseInt(id)` and that might not always return `true` even if there is a match available. In a `forEach` the `return` is useless, so it plays no role, yet the intention is to have the same structure as a `for` loop. – VLAZ Sep 12 '18 at 14:13
  • @vlaz I never said it was, I just said the `if` wasn't needed :) – Michael Curry Sep 12 '18 at 14:22
  • The `return` is also not needed, though. It's an "improvement" that doesn't make any sense as it doesn't change the problem or anything related to it at all. You can also add an extra `if (true === true)` and a `for (let i = 0; i < 1; i++)` to the code with the same success - nothing would actually change. – VLAZ Sep 12 '18 at 14:27

1 Answers1

0

As comments indicate, your return true is not in the right scope. For your purpose, Array.some would work nicely, which returns true if your function returns true for any of the array elements, and then stops processing, saving you some cycles.

ex:

function chartIsVisible2(id) {
    return charts.some(function (chart) {
        if (chart.Id == parseInt(id))
            return true;
    });
}
code_monk
  • 9,451
  • 2
  • 42
  • 41
  • In this case, the callback function can be shortened, as you don't need both the `if` and the `return` - you just need the comparison. Using an arrow function that would be the very short `chart => chart.Id == parseInt(id)` – VLAZ Sep 12 '18 at 14:15