Reputation: 461
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
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
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
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]; }
&&
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)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).
expr1 && expr2
: Ifexpr1
can be converted totrue
, returnsexpr2
; else, returnsexpr1
.
arr=[true];
console.log("your case:",arr[0] && arr[1]);
console.log("1 && 2:", 1 && 2);
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
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
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
Reputation: 1074555
You need to
Start out with logicalAnd
set to true
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