handle multiple checkboxes and complex data structure (reactjs)

user_10566935 0
Viewed 750

Since I am pretty new to React and also not a huge expert on JS especially ES6, I wonder how to make my code (that works) prettier and refactor it.

I want to handle multiple checkboxes in a form and store the current value in the state object. Since the checkboxes are grouped I want a structure as answeredTasks like that:

{  
   "isLoaded":true,
   "answeredTasks":{  
      "1":[  
         "green",
         "blue",
         "red"
      ],
      "5":[  
         "car",
         "cat",
         "house"
      ]
   }
}

Here is my onChange method on a checkbox:

  checkBoxOnChange = e => {
    let currentAnswers = this.state.answeredTasks;

    let element = e.currentTarget;

    // if it is the first answer checked
    if (currentAnswers[element.name] === undefined) {
      currentAnswers[element.name] = [];
    }

    // if uncheck
    if (!element.checked) {
      const index = currentAnswers[element.name].indexOf(element.value);
      currentAnswers[element.name].splice(index, 1);
    } else {
      currentAnswers[element.name].push(element.value);
    }

    this.setState({ answeredTasks: currentAnswers });
  };

Can you tell me how to improve my method, especially with ES6? I mostly struggled with creating the data structure.

Thanks.

reactjs ecmascript-6 object arrays javascript
3 Answers

I preferred I could see your whole code. But if I wanted to suggest you any change:

  1. In general, avoid using "let" in your code and instead use "const". This way, you avoid mutating data which is the industry best practice, and React documentation highly recommends that.

  2. Avoid using splice/push/pop methods which mutate data, and instead use array functions such as filter/map/some/every etc... The methods I mentioned do not mutate data and instead return a referencially different object.

Here is the change you should make:

checkBoxOnChange = e => {
    const currentAnswers = this.state.answeredTasks;
    const element = e.target;

    const newAnswers = !element.checked ?
        (currentAnswers[element.name] || []).filter(val => val !== element.value) :
        (currentAnswers[element.name] || []).concat(element.value);  

    this.setState({ answeredTasks: {...currentAnswers, newAnswers} });
 };

Please note that the {...currentAnswers, newAnswers} inside setState() function is merging the currentAnswers with the new changes in newAnswers. i.e. newAnswers will overwrite the specific object in the outer array. This is the Functional way of doing things.

user_102322690

I'm not the first but here is my version with a test

static changeAnswers = ({target: {name, value, checked}}) => 
  checked
    ? { ...answeredTasks, ...{ [name]: [...(answeredTasks[name] || []), value] } }
    : {
        ...answeredTasks,
        ...{ [name]: (answeredTasks[name] || []).filter(e => e !== value) }
      }

checkBoxOnChange = e => {
  const {answeredTasks} = this.state
  this.setState({answeredTasks: myComponent.changeAnswers(e)})
}


const answeredTasks = { "1": ["green", "blue", "red"] }

const elements = [
  { name: "1", value: "green", checked: false },
  { name: "1", value: "black", checked: true },
  { name: "2", value: "lol", checked: true },
  { name: "2", value: "lol", checked: false }
]

const click = ({ name, value, checked }) =>
  checked
    ? { ...answeredTasks, ...{ [name]: [...(answeredTasks[name] || []), value] } }
    : {
        ...answeredTasks,
        ...{ [name]: (answeredTasks[name] || []).filter(e => e !== value) }
      }

elements.map(element => console.log(click(element)))

user_33155250

Avoid mutation methods splice and push. Use destructuring

const checkBoxOnChange = (ev) => {
  const currentAnswers = this.state.answeredTasks;
  const { name, value, checked } = ev.currentTarget;

  if (!currentAnswers[name]) {
    currentAnswers[name] = [];
  }

  currentAnswers[name] = checked
    ? currentAnswers[name].concat(value)
    : currentAnswers[name].filter((item) => item !== value);

  this.setState({ answeredTasks: currentAnswers });
};
user_78994770
Related