Hakim Asa
Hakim Asa

Reputation: 461

AND multiple parameters

function andMultipleExpr(){
  let logicalAnd;
  let i;
  for (i = 0; i < arguments.length; i++){
    logicalAnd =  arguments[i] && arguments[i+1];
  }
  return logicalAnd;
}

console.log(andMultipleExpr(true, true, false, false));

What I am expecting is to execute this code: true&&true&&false&&false and that should return false.

How to make that work in js? Thanks

Upvotes: 13

Views: 253

Answers (6)

mbojko
mbojko

Reputation: 14679

Early returns should make the code both more efficient and shorter:

function andMultipleExpr() {
  for (let i = 0; i < arguments.length; i++) {
    if (!arguments[i]) {
      return false;
    }
  }

  return true;
}

Upvotes: 5

Archie
Archie

Reputation: 911

Use Array.prototype.every on all the passed arguments to check if they all are true;

function andMultipleExpr(...a) {
  if(a.length === 0) return false; // return false when no argument being passed
  return a.every(Boolean);
}

console.log(andMultipleExpr(true, true, false)); // should return false
console.log(andMultipleExpr(true, true, true)); // should return true

Upvotes: 15

tevemadar
tevemadar

Reputation: 13195

Perhaps you want to hear what went wrong with the loop:

for (i = 0; i < arguments.length; i++){
  logicalAnd =  arguments[i] && arguments[i+1];
}
  1. this loop stores the && of the last two items it encounters. In the ideal case it would && together the last two elements of the array (which is already not what you need)
  2. on top of that at the end of the loop i=arguments.length-1, it will check the last element of the array, and i+1 is the element "after" the last one, which is undefined. In terms of logical relations, it is considered false, but && produces the value itself in such case, and that is why the function returns undefined all the time (this could have been mentioned that in the question).

Docs

expr1 && expr2 : If expr1 can be converted to true, returns expr2; else, returns expr1.

arr=[true];
console.log("your case:",arr[0] && arr[1]);

console.log("1 && 2:", 1 && 2);


Instead, you should use logicalAnd as an accumulator, which collects the result of &&-ing all the previous elements, and a trick what you can use is if the result of a partial && is false, it does not matter what the remaining elements are, the end result is going to be false, so the loop can stop immediately:

function andMultipleExpr(){
    let logicalAnd = arguments[0] || false;
    for (let i = 1; i < arguments.length && logicalAnd; i++){
        logicalAnd = logicalAnd && arguments[i];
    }
    return logicalAnd;
}

console.log("():",andMultipleExpr());
console.log("(false):",andMultipleExpr(false));
console.log("(true):",andMultipleExpr(true));
console.log("(true,true):",andMultipleExpr(true,true));
console.log("(true, true, false, false):",andMultipleExpr(true, true, false, false));

and then you can optimize it towards Archie's answer: the result of &&-ing items is true if all of the items are true, and you do not have to execute a single && operation for calculating the result:

function andMultipleExpr(){
    if(arguments.length===0){
      return false;
    }
    for (let i = 0; i < arguments.length; i++){
      if(!arguments[i]){
        return false;
      }
    }
    return true;
}

console.log("():",andMultipleExpr());
console.log("(false):",andMultipleExpr(false));
console.log("(true):",andMultipleExpr(true));
console.log("(true,true):",andMultipleExpr(true,true));
console.log("(true, true, false, false):",andMultipleExpr(true, true, false, false));

(In the snippets above I aimed for producing false for an empty argument list.)

Upvotes: 2

Nina Scholz
Nina Scholz

Reputation: 386654

You could take Array#every and return the last value.

This approach returns the real result of logical AND &&.

By using the approach, a short circuit is made for the first found falsy value. Then the iteration stops.

function andMultipleExpr(...args) {
    var result; // any return value is configurable for empty args
    args.every(v => result = v);
    return result;
}

console.log(andMultipleExpr(true, true, false, false));
console.log(andMultipleExpr(true, true, 1, 2));
console.log(andMultipleExpr(true, 0, 1, 2));

Upvotes: 2

Patrissol Kenfack
Patrissol Kenfack

Reputation: 863

I think this is a very short way using ES6 Array.prototype.reduce

let andMultipleExpr = (...args) => args.reduce((a, b) => a && b);

console.log(andMultipleExpr(true, true, false, false));

For more explanation about reduce function, please read MDN

Upvotes: 3

T.J. Crowder
T.J. Crowder

Reputation: 1074555

You need to

  1. Start out with logicalAnd set to true

  2. Use logicalAnd when updating it, rather than using two entries from arguments

The minimal change is:

function andMultipleExpr(){
    let logicalAnd = true; // ***
    let i;
    for (i = 0; i < arguments.length; i++){
        logicalAnd = logicalAnd && arguments[i]; // ***
    }
    return logicalAnd;
}
console.log(andMultipleExpr(true, true, false, false));

But mbojko's solution has the advantage of short-circuiting (stopping the loop when it first finds a falsy value), which seems like a good idea.

Since you're using ES2015+, you should probably use a rest parameter rather than arguments, and you can use a for-of loop:

function andMultipleExpr(...flags) {
    let logicalAnd = true;
    for (const flag of flags) {
        logicalAnd = logicalAnd && flag;
    }
    return logicalAnd;
}
console.log(andMultipleExpr(true, true, false, false));

You can also short-circuit that in keeping with mbojko's approach

function andMultipleExpr(...flags) {
    for (const flag of flags) {
        if (!flag) {
            return false;
        }
    }
    return true;
}
console.log(andMultipleExpr(true, true, false, false));

Some people might throw reduce at this, but Archie's every solution is much better. (But since your comparison isn't strict, I'd just make it .every(flag => flag).)

Upvotes: 8

Related Questions