Is it ok to have a function that returns an array of values to be looped by *ngFor loop?

Issue

I have an array of items in my App component and a function that sorts them based on the category:

app.component.ts

 items = [
 { category: 1, type: 'book', price: '100' },
 { category: 2, type: 'electronics', price: '1000' },
 { category: 1, type: 'book', price: '100' },
];


getItemsByCategory(category:number){
  return this.items.filter(item=>item.category === category)
}

In my template I display items using *ngFor:

 <div *ngFor="let item of getItemsByCategory(1)">
  {{item.type}}
</div>

I've heard that because getItemsByCategory returns a new array of objects every time the template is rerendered, it can cause some issues. I understand that this function would execute multiple times, however, if it's very "cheap" to run, I don't see any particular issues with that. Also, I didn't encounter any bugs too while running it in stackblitz.

I'd appreciate if someone could provide more details regarding what bugs this implementation can be causing, and if it's an ok approach in general.

Solution

Unless you're using a non-default change detection strategy, the view is likely re-rendered quite frequently. Every time this happens, your function gets executed. It allocates space for a new array, which garbage collection will need to track and de-allocate later. Then it does the work of filtering the original array to fill the new array with values. With any reasonable number of values, that's not all that much extra work.

By default, *ngFor will always iterate over arrays on a re-render, so you're not adding work from that perspective. And since you're not creating new objects in your array (e.g. via map(item => ({...}))), Angular should still be able to use cached renderings of elements for each of those items.

When it comes to performance questions, consider questions like:

1. How fast is fast enough?
2. What is the cost of making the application faster?
3. What conditions might change to cause acceptable performance to become unacceptable.

In your case, it sounds like you're not noticing any slowness arising from the UI's need to regenerate that array: it's fast enough. Don't make your code needlessly complex to eke out an unnoticeable gain in performance.

That said, the code to make this work faster may not actually be much more complex. You could make the view reference a property that always has the current version of your model. When category or items change, you could update that property by calling your function. If category and items don't change (in your example, both are hard-coded), the code to do this might be simpler than the code you're using now.

And even though your site works fine now, consider what might change to make this more problematic. For example:

• a developer may choose to add expensive logic into getItemsByCategory without realizing how often it gets called.
• a very large number of items could be loaded
• a user might try loading your page on a device with far less processing power than the ones you use for testing.
• events could be added to the user interface which cause change detection (and therefore re-rendering) to happen far more often.

It's wise to consider all of these aspects when deciding whether it's worth the effort to avoid that little bit of extra work. As your application grows, many little, unnoticeable slow-downs can add up to become noticeable. But typically if you're making reasonable efforts to address the larger problems (loading too many items into the UI), you don't need to sweat the small stuff like this.

Answered By - StriplingWarrior