1
\$\begingroup\$

Required to splice specific element value in an array based on the condition.

Both the below code does the same job and yields the correct results. But couldn't figure out how to reduce coding structure with ECMAScript and which is performance oriented considering an array would have atleast 1.5k or even larger

This is with ECMAScript

this.candidateColumns.filter(item => item === 'Certificate').map(m => {
    if (!this.showControlsBasedOnPrivileges('CERTIFICATEPRINT')) {
        this.candidateColumns = this.candidateColumns.filter(item => item !== 'Certificate')
    }
});

This is simple forEach Loop code

this.candidateColumns.forEach((currentValue, index) => {
    if (currentValue === 'Certificate' && !this.showControlsBasedOnPrivileges('CERTIFICATEPRINT')) {
        this.candidateColumns = this.candidateColumns.filter(item => item !== 'Certificate')
    }
});

Any other Built In function Approach could be done for better performance or readability

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

.map is only for the construction of new arrays - for example, arr.map(num => num / 2), to transform every element of one array into another array. If you aren't returning anything from the .map callback, or if you aren't using the resulting array, then .map isn't appropriate. If all you're interested in is something that can be accomplished with side-effects, use a generic iteration method like for..of or forEach (like with your second snippet).

But:

Select the proper iteration method - forEach can be used to accomplish pretty much any task while iterating over an array, but that's somewhat of a weakness, because it makes the intent of the code somewhat less clear. In this case, your logic is:

If CERTIFICATEPRINT showing is off, remove any Certificate items from the candidateColumns array.

There's no need for a nested loop, which will increase the computational complexity to O(n ^ 2) (a big jump). This can easily be accomplished by putting a .filter inside an if, O(n):

if (!this.showControlsBasedOnPrivileges('CERTIFICATEPRINT')) {
  this.candidateColumns = this.candidateColumns.filter(item => item !== 'Certificate')
}

performance oriented considering an array would have atleast 1.5k or even larger

With thousands of elements, O(n ^ 2) would indeed be a problem, but the O(n) operation should be quite reasonable; a few thousand operations is next to nothing on modern hardware.

Is reassignment necessary? You assign the result of the filter to this.candidateColumns. Is that required? Could you, for example, allow the filtered array to be used by returning it instead, or by passing it along to whatever needs the data? If possible, consider doing so, if it doesn't make the code convoluted - code is generally easier to make sense of when mutations are only done when necessary.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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