10

I am working on angular2 application in which my team member are using getter and setter to set input properties as follow

private _showModal;
@Input() set showModal(showModal){
     this._showModal = showModal;
}
get showModal() {
     return this._showModal;
}

but I am not sure its a good way to do this. I thought one should use getter setter in the case where dev have to do some validation or check or do some other function while setting or getting value

Rhushikesh
  • 3,630
  • 8
  • 45
  • 82
  • If there is no more logic than this, your colleague is probably a .NET developer. Do they also create empty constructors and sprinkle public access modifiers everywhere? – Lars Gyrup Brink Nielsen Dec 13 '19 at 06:28

5 Answers5

13

Where we do the reading(get) and writing(get) majorly determines how much cost we pay for performance.

setters

A setter function is called every time we write some value.

Now we generally do the writing part that calls the setters in our TypeScript Classes. So they don't get called so often unless there's a set operation, which isn't quite frequent in general.

getters

A getter function is called every time we read some value.

getters are generally called in templates in different data binding syntax like string interpolation({{}}), property binding([---]=""), attribute binding([attr.---]=""), style binding([style.---]="") etc.

Now the catch with this is, every time Angular performs change detection, the getters get called. It's fine as long as there isn't much logic in your getter. But that still leaves a room for newer developers in the team to add logic in there without being aware of the performance hits that it's gonna create.

So in summary, from what I understand, it's okay to have setters. But having getters and it's cost on performance would mainly depend on where those getters are used. If they're used in one of the template binding syntaxes, then it's safe to NOT HAVE THEM IN THE FIRST PLACE. If they're not being used in the template, it's okay to have them.

I've actually written an article and a few answers on various StackOverflow Threads that you might want to check out as well. So I'm adding them as a list below:

  1. Angular: Prevent DomSanizer from updating on DOM Events

  2. Angular performance: ngStyle recalculates on each click on random input

  3. Angular 7 ,Reactive Form slow response when has large data

  4. Angular Performance: DOM Event causes unnecessary function calls

  5. I changed my implementation of an EXTREMELY deeply nested Angular Reactive Form and you won’t believe what happened

Hope this gives you some perspective. :)

Community
  • 1
  • 1
SiddAjmera
  • 38,129
  • 5
  • 72
  • 110
  • I was thinking exactly this when I googled "angular getter bad practice". I'm considering adding a lint rule to forbid `getters`, I have seen performance issues happening few times in my project because of this. – Alisson Reinaldo Silva Feb 24 '21 at 23:16
3

avoid getters specifically if possible. Getters have a negative effect on change detection. In order to know whether the view should be updated, Angular needs to access the new value, compare it with the old one and make the decision on whether the view should be updated. For this reason, the value is compared and updated on every change detection cycle. If for some reason you need to transform data when an @Input() is changed in a component a setter is fine for example.

value;
@Input('myValue') set myValue(v) {
 transformValue(v);
}
transformValue(v) {
...*sometransform*
this.value = transforrmedValue
}

in this example, a setter is placed on the Input and the value is transformed every time a new myValue is pushed to the component. However, if a getter is introduced the component will check the getter every time change detection cycles anyway. You could also use other things like pipes or ngOnChange instead of setters.

UPDATE

as long as your component is using ChangedetectionStategy.OnPush now using a getter is fine but still avoid it if you are not using OnPush

Vinez
  • 560
  • 2
  • 11
  • We have a software running Angular 12 and in my tests I could not confirm your claim. I have a service with a public list and a component displaying its values. Then I added a setTimeout() to the constructor of the service, changing the list. If I switch from a public list to private list with agetter, it still gets updated in the view automatically. (Edit: without OnPush) – Panossa Mar 07 '23 at 14:35
  • I am unsure what you mean OnPush is a change detection strategy. When active the component will only update if certain criteria are met (an input change or async pipe triggers the page). Without it any change will trigger an update. By the description you gave in your statement that is the behavior I would expect and seems mostly unrelated to my solution. – Vinez Mar 08 '23 at 15:48
  • So did I get it right that getters create no problems unless you're using OnPush on an older version of Angular? We use Default most of the time and are about to switch to using getters and setters in every service. Even when those getters are used in `@Inputs` and/or `@Outputs` of components. So far we've noticed no problems but your post worried me. – Panossa Mar 08 '23 at 17:02
  • No the opposite. Tell you what, lets do an experiment. In one of your components add a getter without on push. Put a console log in the getter. Reference that getter somewhere on your template for that component. You will notice the console log triggers multiple times a second. Thats because without OnPush angular has no way of knowing whether the getter needs to be called again or not and rerender the component. This is true not just for getters but any method that is called from a template. This will cause massive performance issues in your application. – Vinez Mar 09 '23 at 14:31
  • Oh, I didn't know this is a general problem with methods in components! Damn, good to know. This means services are allowed to have methods/getters, components too, unless they're used in the UI? So a performant solution would fetch data from a service and save it to a public variable in the view, then update it... somehow? – Panossa Mar 10 '23 at 16:51
  • Actually, @Vinez, how do I know this is actually a thing with methods? I don't know how to prove the same component is not loaded many times without any code in the template, right? I have to prove this to my co-workers ^^' – Panossa Mar 10 '23 at 17:00
2

This is part opinion, part the requirements of your application. It is certainly not bad to use getters and setters. But I would also use them with discretion and in most cases getters and setters may be unnecessary.

In the example code you provide, there is no reason to use a getter and setter. You are correct in that it can help when doing some sort of validation check or when something else in contingent upon the value being set, etc. For example, maybe you need to call some function when an @Input() property changes value, this can be an easy way to accomplish that. But in many cases, this is probably not a requirement for every variable/input in your application.

Hope this helps.

Tyler Jennings
  • 8,761
  • 2
  • 44
  • 39
0

I would only do this if you alter _showModal in the get or set.

Using it like your team member does (with a backing property) just adds more lines of code.. In that case I would make a public showModal property.

I don't know if there is a "best practice" for this. I think it mostly has to do with personal preference.

Mike Bovenlander
  • 5,236
  • 5
  • 28
  • 47
-3

It is a good practice as you have more control on your data. Especially when you need to know when your data has changed with angular change detection.

Most of the time, I use them in my services to share the data over several components.

They also interact well with observables to avoid calling several times an endpoint to get your data.

Alex Beugnet
  • 4,003
  • 4
  • 23
  • 40
  • I use them in my services to share the data over several components. They also interact well with observables to avoid calling several times an endpoint to get your data. Any example for that ? – Bhavya Sanchaniya Aug 01 '18 at 07:22