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 break
ing inside a for
loop to check if an item passes a condition, it would be better to use .some
. Also, rather than .push
ing 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
?