0

In Angular, sometimes when using *ngIf, you have to check a bunch of conditions whether to render the element or not. Sometimes it becomes a bit harder to read. Is using getter methods for cases like this a bad idea? Why or why not? Should they be completely avoided?

I have tried storing the conditions in a variable before for readability but I encountered problems (I was checking the values of the properties of the class), I'm not sure but I think it had something to do with outdated data, they're not returning the correct result but when I used getters, the DOM gets updated when something changes, basically I get the correct results.

Something like:

In the view:

<div [ngClass]="{ 'hidden' : isFlagUnchecked}"></div>

In the component class:

get isFlagUnchecked(): boolean { return !this.profileForm.get('flag').value; }

OR: (An example of grouped conditions, basically looking for ways so they can be further simplified or encapsulated with a more descriptive name, like storing in a single variable, or if they should be)

<div *ngIf="!data['content_id'] || !data['content_name']" class="container"></div>
herondale
  • 729
  • 10
  • 27
  • 2
  • 1
    It would be helpful to see a concrete code example, but either way, I suspect this will be too opinion-based – CertainPerformance Mar 08 '19 at 03:06
  • I didn't think a code snippet was necessary, but I updated my question @MadhawaPriyashantha – herondale Mar 08 '19 at 03:10
  • I see no problem with having the conditions in the `*ngIf` itself. But as u/CertainPerformance says, it's opinionated. I don't like the idea of maintaining one variable for visibility of some element, because you'd have to update this every time some dependent variable changes and this will be even less maintainable. Getters is a good practice too. IMO. Especially if your logic contains a number of different objects and criteria. – Shashwat Black Mar 08 '19 at 03:14
  • @ShashwatBlack Oh, I see. I was just wondering for readability of the code, if it would be simpler to read it if grouped conditions like that are encapsulated in a getter with a descriptive name, or if it doesn't make much of a difference and actually only hurts the performance of the application. – herondale Mar 08 '19 at 03:15
  • 4
    Highly opinion based but I find getters to be more readable and easier to maintain rather than complex logic in templates. A rule I’ve always followed as long as templatig engines have existed is keep them as simple as possible. – bryan60 Mar 08 '19 at 03:16
  • @bryan60 Thanks for your input, I was exactly wondering about that. Right now I prefer getters too because they make the template easier to read. – herondale Mar 08 '19 at 03:18
  • @herondale If these conditions are at a point of hurting your app performance, you should seriously consider restructuring your app to be more hierarchical, with each hierarchy with it's own set of conditions. But yeah, I don't think they are any less readable. Less pretty for sure, but just as readable. – Shashwat Black Mar 08 '19 at 03:19
  • Np. If you were wondering, there is not a performance trade off between the two. They perform the same. – bryan60 Mar 08 '19 at 03:20
  • Something to be add to this is how functions on html templates interact with Angular's change detection. I have seen the performance of a page slow down drastically from template functions being called numerous via Angular's change detection. SO post that elaborates more: https://stackoverflow.com/questions/49320756/how-angular-change-detection-is-triggered-when-you-are-binding-to-a-function – Rod Mar 08 '19 at 04:04

1 Answers1

0

Change your getter property to

get isFlagUnchecked(): boolean { console.log('Checking flag'); return !this.profileForm.get('flag').value; }

and see how often that getter is hit.

It all depends on what else your page is doing and how often change detection is being triggered.

It is always going to be better performance to have a boolean property that gets updated with a change event handler on the flag input of the profileForm. Something like

<input name="flag" (change)="updateIsFlagUnchecked()">

This way the logic only gets run on changing the form field rather than continually on change detection.

Adrian Brand
  • 20,384
  • 4
  • 39
  • 60