Improving a filter array function, to match elements and conditions

Question

I have to do some check/match elements with conditions. A have a list of conditions, each with a specific ID. I've put for each of my elements an array with the ids of their respective affirmative conditions.

Some elements can share a condition. The goal is to, everytime a condition is "checked" by the user, the code filter the array with all elements, showing only the ones that attend the conditions.

I've made this function. But I don't know if its the "best" way..

this.selectedConditions is already maped by ID

Any help?

---

private filterElements(){

    let filtered: any[] = [];

    originalArray.forEach( item => {

      let isPossibility: boolean = true;

      for (let i = 0; i < this.selectedConditions.length; i++) {

        if( item.condition_matrix.indexOf( this.selectedConditions[i] ) === -1 ){
          isPossibility = false;
          break;
        }
      }

      if( isPossibility ){
        filtered.push(item);
      }

    });

    this.outputArray = filtered;

    if(this.outputArray.length === 0){
      this.outputArray.push( { <a_filler_object> } );
    }
}

---

Elements list example:

elements: [
    {
        name: 'el1',
        condition_matrix: [1,2]
    },
    {
        name: 'el2',
        condition_matrix: [2,3]
    },
    {
        name: 'el3',
        condition_matrix: [2,1]
    }
]

Conditions list example:

conditions: [
    {
        id: 1
        name: 'cond1' 
    },
    {
        id: 2
        name: 'cond2' 
    },
    {
        id: 3
        name: 'cond3' 
    }
]

Answer 1

It looks like you're using TypeScript, so a couple pointers:

Avoid any - any effectively disables type-checking, which defeats the whole point of using TypeScript. Better to type variables properly:

let filtered: Array<{ name: string; condition_matrix: Array<number>; }> = [];

Or, even better, let TypeScript infer the types automatically when possible, by transforming the data all at once with array methods (see below). No need to denote types when TS can infer them automatically - eg let isPossibility: boolean = true; can be just let isPossibility = true;

Avoid let, prefer const - see here, using const indicates that the variable will never be reassigned, which reduces cognitive overhead.

Semantic array methods While for loops work, they should only be used as a last resort, when other array methods aren't appropriate. Here, rather than breaking inside a for loop to check if an item passes a condition, it would be better to use .some. Also, rather than .pushing to an outer array based on the condition, use .filter instead:

this.outputArray = originalArray.filter(
  item => this.selectedConditions.some(
    condition => item.condition_matrix.includes(condition)
  )
);

(Better to use .includes than to use an indexOf check against 0)

Or:

Time complexity If performance is an issue and you have a lot of conditions, rather than iterating all of the conditions each time, consider constructing a Set of all of the selectedConditions first; Set#has is O(1), which is an improvement over iterating over arrays with O(n).

const allConditions = new Set(this.selectedConditions);

Conditions list typo? The data structure in your code is an array of objects, but you're using item.condition_matrix.indexOf( this.selectedConditions[i] ). Did you mean:

item.condition_matrix.indexOf( this.selectedConditions[i].id )

?

(If so, and you want to use the Set method, map to IDs first:

const allConditions = new Set(
  this.selectedConditions.map(({ id ]) => id)
);

)

Side-effects? The result of the whole function is to set a value (to this.outputArray). Is this required? Code is generally easiest to understand and reason about when it does not contain side-effects. Imagine seeing some code you haven't looked at before, and compare:

this.calculate();
console.log('Result:', this.result);

vs

console.log('Result:', this.calculate());

The second is easier to make sense of because the method is used for its return value instead of for its side-effects (which, to understand, you'd have to read through the method to find).

Could your filterElements method return a value instead of assigning to this.outputArray?