Question

I've always been taught that having side-effects in an if condition are bad. What I mean is;

if (conditionThenHandle()) {
    // do effectively nothing
}

... as opposed to;

if (condition()) {
    handle();
}

... and I understand that, and my collegues are happy because I don't do it, and we all go home at 17:00 on a Friday and everyone has a merry weekend.

Now, ECMAScript5 introduced methods like every() and some() to Array, and I find them very useful. They're cleaner than for (;;;)'s, give you another scope, and make the element accessible by a variable.

However when validating input, I more-often-than-not find myself using every/some in the condition to validate the input, then use every/some again in the body to convert the input into a usable model;

if (input.every(function (that) {
    return typeof that === "number";
})) {
    input.every(function (that) {
        // Model.findById(that); etc
    }
} else {
    return;
}

... when what I want to be doing is;

if (!input.every(function (that) {
    var res = typeof that === "number";

    if (res) {
        // Model.findById(that); etc.
    }

    return res;
})) {
    return;
}

... which gives me side-effects in an if, condition, which is bad.

In comparison, this is the code would look with with an old for (;;;);

for (var i=0;i<input.length;i++) {
    var curr = input[i];

    if (typeof curr === "number") {
        return;
    }

    // Model.findById(curr); etc.
}

My questions are:

  1. Is this definitely a bad practice?
  2. Am I (mis|ab)using some and every (should I be using a for(;;;) for this?)
  3. Is there a better approach?

No correct solution

Licensed under: CC-BY-SA with attribution
scroll top