Get value from dynamically generated inputfield [closed]

I have an object:

schedule = {
      'Monday': { morning: { from: "9:00", to: "12:00" }, afternoon: { from: "15:00", to: "17:00" } },
      'Tuesday': { morning: { from: "9:00", to: "12:00" }, afternoon: { from: "15:00", to: "17:00" } },
      'Wednesday': { morning: { from: "9:00", to: "12:00" }, afternoon: { from: "15:00", to: "17:00" } },
      'Thursday': { morning: { from: "9:00", to: "12:00" }, afternoon: { from: "15:00", to: "17:00" } },
      'Friday': { morning: { from: "9:00", to: "12:00" }, afternoon: { from: "15:00", to: "17:00" } },
    };

I transform the schedule object into an array so that Angular can iterate over it this way:

getScheduleArray() {
    let profile = this.profile;
    var days = Object.keys(profile.schedule);
    var array: { day: string; schedule: any; }[] = [];
    days.map(function (key) {
      return array.push({ day: key, schedule:profile.schedule[key] });
    });
    return array;
  }

In my Angular application I have many different input fields that are dynamically generated this way:

<div class="p-2" *ngFor="let day of getScheduleArray()">
    <p><b>{{day.day}}</b></p>
    <div class="row">
        <p class="col">Morning</p>
        <div class="col"><input class="form-control" type="text" placeholder="From" [(value)]="day.schedule.morning.from" (input)="updateSchedule(day.day, 'morning', 'from', $event)"></div>
        <div class="col"><input class="form-control" type="text" placeholder="To" [(value)]="day.schedule.morning.to" (input)="updateSchedule(day.day, 'morning', 'to', $event)"></div>
    </div>
    <div class="row">
        <p class="col">Afternoon</p>
        <div class="col"><input class="form-control" type="text" placeholder="From" [(value)]="day.schedule.afternoon.from" (input)="updateSchedule(day.day, 'afternoon', 'from', $event)"></div>
        <div class="col"><input class="form-control" type="text" placeholder="To" [(value)]="day.schedule.afternoon.to" (input)="updateSchedule(day.day, 'afternoon', 'to', $event)"></div>
    </div>
</div>

To update the schedule I use the following method:

updateSchedule(day: string, partOfDay: string, startOrFinish: string, event: any) {
    this.profile.schedule[day][partOfDay][startOrFinish] = event.target.value;
  }

Is there any better way to achive the same result?

Solution 1:

Some things that I would improve:

1. Don't use var at all. While it works it's there to support legacy JS apps and you should always use const or let. let is for variables that you plan to update later such as a counter. const is for variables that will never change value such as temporal variables.

2. Use const whenever possible. Most of the times we need a temporal variable to make our code more readable. The typescript compiler is smart enough to search and inline const variables to make our code faster. If you're not going to update a variable use const

3. Use map functional predicate properly. In this case you're creating a side effect which makes it no different as just writing a for (and in this case it would be just better)

Here you're doing:

    var array: { day: string; schedule: any; }[] = [];
    days.map(function (key) {
      return array.push({ day: key, schedule:profile.schedule[key] });
    });
    return array;

And it would perform better if you did:

    var array: { day: string; schedule: any; }[] = [];
    for(let key of keys) {
      array.push({ day: key, schedule:profile.schedule[key] });
    });
    return array;

To properly use the map predicate you should not mutate the external array and you could do:

  return keys.map(key => ({ day: key, schedule:profile.schedule[key] }))

What map does is that it takes an original array and runs a function though every single element and aggregates that to the final result.

4. Cache your result, it's making your app slow. When we're a little bit lazy and we just get the value of a function into the angular template we're risking to create new objects (you're creating arrays every time the function is called). That ends up in the change detector running several times and running the code inside the function every time because you're always returning a new instance of an array (with identical contents, that's why we dont notice changes in the GUI)

To do just that you can just run the code on ngOnInit and store it on a property from your component:

@Component({...})
export class YourComponent implements OnInit {
  yourSchedule: Array<{ day: string; schedule: any; }>
  ngOnInit() {
    this.yourSchedule = this.getScheduleArray()
  }
}

This way your code is run once and it's just faster overall.

5. Finally: Use reactive forms. Your code works as is but you have very little control to do input validation. You'd need to modify your getScheduleArray() and somehow return a FormGroup or FormArray with the required validators. This is completely optional as it depends on the requirements of your projects but I just wanted to showcase what else you could switch on this. :)

Overall: great work :)!

Solution 2:

As far as I understand, you are just updating the value of the schedule object. If you are using Angular 2+, then I would advise using [(ngModel)] dire by importing the Forms Module into your component module.

If the data needs to be written to the properties of the this.profile.schedule object, then the code will look something like this:

...
<div class="col"><input class="form-control" type="text" placeholder="From" [(ngModel)]="profile.[day.day].schedule.morning.from" /></div>
...

However, note that there is two-way data binding here. The initial value is taken from the object and written to the object. details

Therefore, it may be worth changing the data in the array of values you have created. When you click the submit button, write the changed data to the main object