if condition inside forEach's anonymous function does not work

Question

Consider the code snippet below:

function isUniform(myArray) {
    myArray.forEach(function(element) {
        if (element !== myArray[0]) {
            return false;
        }
    });

    return true;
}

The intention is that the function should take an array as input and return true if all the elements in the array are the same and false otherwise.

eg:

isUniform([1,2,1,1]); // should return false isUniform([1,1,1,1]); // should return true

However, the if condition:

if (element !== myArray[0])

never seem to be true in the case of isUniform([1,2,1,1]).

What is it that I am missing ?

why are you using `return` in a `forEach()`? that doesn't makes sense because forEach() doesn't return... Use `[].some()` and return the some() call, or do the extra iteration and a closure var and stick with forEach... — dandavis, Jul 08 '16 at 19:24
well, it's good to see new comers use `forEach` instead of `for(`, you're on the right track. — dandavis, Jul 08 '16 at 19:27
@dandavis So the only purpose of `forEach` is to perform side effects on its `Array`? Haven't used it. Won't use it. — , Jul 08 '16 at 19:32
you can use the 2nd argument to [].forEach to inject a destination object instead of using side-effects, but few people use that, opting instead for [].map/[].some/[].each/[].filter — dandavis, Jul 08 '16 at 19:33
if you use some/every, make sure to slice off the first element, the one you are comparing. — dandavis, Jul 08 '16 at 19:49
@Barmar: How can you mark this as a duplicate? I saw the question you marked and it looks different. My question is specific to a code snippet, although the underlying answer may be the same. — nitin_cherian, Jul 08 '16 at 19:58
I mark it as a duplicate because the answer there explains the problem here as well. The questions may not be exxactly the same, but the underlying ideas are. — Barmar, Jul 08 '16 at 19:59
@Barmar: That is bad!. See the discussions this question has garnered. — nitin_cherian, Jul 08 '16 at 20:00

score 12 · Accepted Answer · answered Jul 08 '16 at 19:47

So the return true isn't returning a value for the function isUniform, it's returning a value for the callback that you provided to the forEach method. forEach is really only used to create side effects. So forEach executes the callback on each element, sees that the callback returns false, but then doesn't have anything to do with that value, so it throws it out and moves on to the next element in the array. After iterating through the array, it moves on to the next line of code and returns true for the function.

One way that you might do this using forEach is to declare a variable that you initialize as true and manipulate that within the callback. This is necessary, as there's not a way to end the execution of a forEach loop early. So you might instead use:

function isUniform(myArray) {
    var passing = true;
    myArray.forEach(function(element) {
        if (element !== myArray[0]) {
            passing = false;
        }
    });

    return passing;
}

Or you could use a for loop or a for-of loop, in which case the return statement would work as you had originally expected. You're probably already familiar with for loops. For-of loops were introduced in ES2015 (so they might not work on all JS engines). A for-of loop would look like this:

function isUniform(myArray) {
    for (element of myArray) {
        if (element !== myArray[0]) {
            return false
        }
    }
    return true
}

However, the best way to do this is probably using the built in array method every, which returns true if every element in the array passes the test provided in the callback. So you might test every element to see if they're equal to the 0th element in the array and thus equal to each other:

function isUniform(myArray) {
    return myArray.every(function (currentElement,index,array) {
        return currentElement === array[0]
    })
}

That's short enough that you really don't even need to put it in its own function -- and your code will probably be more readable if you don't.

Docs: Array.prototype.every: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/every

For-of loop: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of

@dandavis You're referring to the `array.every()` block? I'm pretty confident that it works fine, and I've tested it in a repl. What circumstances did you get it to fail under? — svangordon, Jul 08 '16 at 19:58
wow, i'm just all kinds of dumb today. you're right, it's fine and i should read the code (function names included) more next time. sorry about the confusion. — dandavis, Jul 08 '16 at 20:01
Please, use arrows: `const isUniform = xs => xs.every(x => x === xs[0])` — , Jul 08 '16 at 20:04
@dandavis You make a fair point. Although this renders my answer useless. In any case this answer contains more info so probably more helpful to future stackers. — Richard Barker, Jul 08 '16 at 20:05
@LUH3417 Arrow functions aren't supported everywhere, at least not yet. I figured that if the asker doesn't understand why this bit of code doesn't work, they probably don't understand lambdas. But yes, I certainly agree that arrow fn's would make this easier to read and write — svangordon, Jul 08 '16 at 20:10
@LUH3417 First of all, I wrote the code to help the asker better understand JS. It's not like I'm going to ship this tomorrow, and then have to support it in the future. This is a trivial problem, with a trivial example. Furthermore, it's not like this code is going to break. One of the principles of Ecma is "Don't break the web". Just because ES6 has introduced syntactic sugar doesn't mean that older syntax is deprecated or doesn't work. In fact, there are plenty of places where ES6 features, like lambdas, *won't* work without transpiling, like older Node versions or older web browsers. — svangordon, Jul 10 '16 at 18:28

if condition inside forEach's anonymous function does not work

1 Answers1

Linked