##Main Question
Can somebody please tell me what would have made my code more "correct"?
It is difficult to know exactly what would have made my code more "correct" but it does look like the method handleClick
is more complex than it needs to be. Specifically, there is a for
loop that is not needed. Perhaps you were attempting to guard against cases where name
is not actually a property on data
. That could be achieved using Object.hasOwnProperty() - for example, the for
loop could be replaced with this block of code:
if (data.hasOwnProperty(name)) {
data[name] = `;
}
###Useless re-assignment of data
in handleClick
method
At the beginning of the handleClick
method, I see this line:
let data = this.state.data;
And then after storing constants for the event target and its name, there is this line:
data = this.state.data;
That line seems to serve no purpose.
###Input type?
Are there other types of inputs besides checkboxes? If not, then the following line in handleClick()
seems too complicated:
const value = target.type === 'checkbox' ? target.checked : target.value;
There appear to be no value attributes set on any elements.
###Binding the method to the event handler
The handleClick
method is called from an arrow function in the onChange event handler attribute:
onChange={(event) => this.handleClick(event)}
According to the ReactJS documentation:
The problem with this syntax is that a different callback is created each time the [component] renders. In most cases, this is fine. However, if this callback is passed as a prop to lower components, those components might do an extra re-rendering. We generally recommend binding in the constructor or using the class fields syntax, to avoid this sort of performance problem.1
There is no need to add an extra anonymous function just to call that method. Bind the method directly, like below:
onChange={this.handleClick}
###Checked attribute The checked attribute of each checkbox input is set according to the following:
checked={this.state.data[key].value}
But there is no need to use .value
- it can be simplified like below:
checked={this.state.data[key]}
1https://reactjs.org/docs/handling-events.html