0
\$\begingroup\$

Currently I have this code

function foo(firstId, secondId){
    var boolOne = false;
    var boolTwo = false;

    for (var i = 0; i < arr.length; i++) {
        var current = arr[i];
        var id = current.id;

        if(firstId == id){
            boolOne = true;
        }

        if(secondId == id){
            boolTwo = true;
        }

        if(boolOne && boolTwo){
            break;
        }
    }

    if(boolOne && boolTwo){
        bar();
    } else {
        // throw error
    }
}

I want to check if two objects with the given parameter ids already exist in the array.

I put my code into one for-loop but I could also split the code into two loops and break on a match.

I know I could also use

var boolOne = arr.includes(item => item.id == firstId)

but I have to support the Internet Explorer 9+. Is there something that can be optimized?

\$\endgroup\$
  • \$\begingroup\$ sorry, IE 9 and later \$\endgroup\$ – hrp8sfH4xQ4 Sep 11 '18 at 13:26
  • 1
    \$\begingroup\$ What do you mean by optimize? If it´s running faster, your solution is perfect. If you mean code length, it could be reduced but your solutions is clear enough already. \$\endgroup\$ – juvian Sep 11 '18 at 14:27
  • \$\begingroup\$ To call this 'perfect' is a real stretch.. \$\endgroup\$ – konijn Sep 11 '18 at 14:41
  • \$\begingroup\$ @konijn perfect performance wise ^^. Your solution fixes all other things :) \$\endgroup\$ – juvian Sep 12 '18 at 16:23
2
\$\begingroup\$

From a short look

  • foo is a terrible name
    • If this is not the real name, then please do submit the real code
  • arr seems to be a global, bad practice
  • You don't need current, you could just go for var id = arr[i].id;
  • For older browsers, it is faster to cache arr.length instead of retrieving it every time
  • bar is a terrible name, I really hope you don't write production code like that
  • the call to bar should probably happen outside of that function

Given that, I would go for the below pseudo code:

function arrayHasBothObjects(list, id1, id2){
    var boolOne = false,
        boolTwo = false,
        size = list.length;

    for (var i = 0; i < size; i++) {
        var id = list[i].id;

        boolOne = boolOne || id == id1;
        boolTwo = boolTwo || id == id2;

        if(boolOne && boolTwo){
            return true;
        }
    }
    return false;
}
\$\endgroup\$
-1
\$\begingroup\$
  • You can solve it using functional approach, which makes code more consice.
  • I suggest you to try solve more general problem with n identifiers, because arguments names like 'id1' or 'firstId' is a bad smell.

My solution:

var hasBoth = function(list, id1, id2) {
    return [true, true].toString() === list.reduce(function(res, x) {
        return [id1 === x.id || res[0], id2 === x.id || res[1]];
  }, [false, false]).toString();
}
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.