addControls in a for loop (Angular)

1

Hello,

This is my code:
…component.ts:

test = [
    'test1',
    'test2',
    'test3'
  ];

  public mainForm : FormGroup;

  constructor() {
    this.mainForm = new FormGroup({

    }, {
      updateOn: 'change'
    })

    let f = new FormGroup({
      fromx: new FormControl('',[]),
      tox: new FormControl('',[]),
    },{
      updateOn: 'change'
    });

    for(let t in this.test){
      this.mainForm.addControl(this.test[t]+'_s',f);
    }
  }

  testF() {
    console.log(this.mainForm);
  }

…component.html:

<form *ngFor="let t of test"
[formGroup]="mainForm.get(t+'_s')"
autocomplete="off"
>
  <div [formGroup]="mainForm.get(t+'_s')">
    <input 
    type="text"
    matInput formControlName="fromx" placeholder="from" required/>
    <input 
    type="text"
    matInput formControlName="tox" placeholder="from" required/>
  </div>
</form>
<button (click)="testF()">test</button>

My problem is: After I fill the fields, and I send the form values, all controls receive the same value(the latest value tipped in the input)…I noticed that the problem is coming from the for loop, but I do not understand why, and how can I do it work.

2

Hi @maba welcome to the FCC forums :smile:

There are a few code smells which I’ll go over after addressing the underlying problem:

The reason your seeing the same value in multiple formGroups is your using the same formGroup.

// f is the same form group
let f = new FormGroup({
      fromx: new FormControl('',[]),
      tox: new FormControl('',[]),
    },{
      updateOn: 'change'
    });

    for(let t in this.test){
      // you add the f formGroup to the mainForm multiple times
      this.mainForm.addControl(this.test[t]+'_s',f);
    }

You probably want to create a new FormGroup within the loop, so each iteration, and thus each “sub control” will have its own values/reference.

Beyond the underlying problem I recommend a few things.

  1. Don’t add random spacing if it isn’t neccessary:
this.mainForm = new FormGroup({

    }, {
      updateOn: 'change'
    })

should be:

this.mainForm = new FormGroup({}, {updateOn: 'change'})

Or use something like prettier to auto-format your code for you.

  1. Don’t use let
    let usually can be replaced with const 
let f = new FormGroup(/**/)

could be:

const f= new FormGroup(/**/)

  1. Don’t use 1 name variable names as its to easy to get it confused.
    f should be form, and t test. It makes reading the code obvious to you, and anyone else who reads it down the line. This helps once the code starts getting more complex.

  2. Don’t use the constructor method in components in Angular to initialize variables, use ngOnInit
    here’s the docs for the method. ngOnInit is the method called by Angular when the component is created, where-as the constructor is used by JS when the class is created. Its better to initialize stuff in ngOnInit as Angular is handling the method and component at that point, rather than some lower level JS concept. Generally it wont matter, but its best practice to leverage ngOnInit.

3

Thank you a lot for your response and your advices ! :grinning: