2

I'm making simple To Do List app,Everything is working.I just want to make sure I'm doing it right without any mistakes. I'm concerned about Check box update part,Please check the code and tell me if I'm doing anything wrong.

Here is the put method for Checkboxes

checkBoxRouteUpdate = () => {
    let {todos} = this.state
    let newArray = [...todos]
    axios
        .put(`http://localhost:8080/checkEdit/`, {
            checked: newArray.every(todo => todo.checked)
        }).then((res) => {
        console.log("res", res);
    })
        .catch((err) => {
            console.log("err", err);
        });
}

checking all of them

checkAllCheckBox = () => {
    let {todos} = this.state
    let newArray = [...todos]
    if (newArray.length !==0) {
        newArray.map(item => {
            if (item.checked === true) {
               return  item.checked = false
            } else {
               return  item.checked = true

            }
        })
        this.checkBoxRouteUpdate()
        this.setState({todos: newArray})

    }
}

Checking single Check Box

 checkSingleCheckBox = (id) => {
        let {todos} = this.state
        let newArray = [...todos]
        newArray.forEach(item => {
          if (item._id === id) {
              item.checked = !item.checked
              axios
                  .put(`http://localhost:8080/edit/${id}`,{
                      checked:item.checked
                  })
                  .then(res => {
                      this.setState({todos: newArray})
                      console.log('res',res)
                  })
                  .catch((err) => {
                      console.log("err", err);
                  });
          } else {
          }
        })

    }

Deleting Only Checked Items

deleteAllChecked = () => {
    const todos = this.state.todos.filter((item => item.checked !== true))
    axios
        .delete('http://localhost:8080/deleteAllChecked')
        .then((res) => {
            this.setState({ todos,
                pageCount: Math.ceil(todos.length / 10)})
            console.log("res", res);
        })
        .catch((err) => {
            console.log("err", err);
        });
}
2
  • 1
    Since you do not mutate the state array, you can remove the let newArray = [...todos]. You only need to do that if you update the array and put it back into the state. Commented Jun 13, 2020 at 21:19
  • Thank you for the correction Commented Jun 13, 2020 at 21:20

2 Answers 2

2

You can check/uncheck them another way

  this.checkBoxRouteUpdate()
  this.setState(state => ({
    ...state,
    todos: state.todos.map(todo => ({
      ...todo,
      checked: !item.checked
    }))
  }))

I think you should delete after api returns ok status

.then((res) => {
  this.setState(state => {

  const todos = state.todos.filter((item => item.checked !== true));

  return {
    ...state,
    todos,
    pageCount: Math.ceil(todos.length / 10)
  }
})

Sign up to request clarification or add additional context in comments.

1 Comment

Yes, you might have different objects in state and that's why im using spread operator to keep them.
1

I add a lot of comments, some of these some just another way to do what you do and others are personal preferences, but the most important is that you can see alternatives ways to do things :).

checkBoxRouteUpdate = () => {
  const todos = [...this.state.todos]  // Better use const and initialize the array of objects directly

  /*since you will use this array just in one place, is better if you iterate in 
    the [...todos] directly without save it in a variable
  
    let newArray = [...todos]  
  */

  axios
    .put(`http://localhost:8080/checkEdit/`, {
      checked: todos.every(({checked}) => checked) // here you can use destructuring to get checked
    }).then((res) => {
      console.log("res", res);
    })
    .catch((err) => {
      console.log("err", err);
    });
}
```

checking all of them
```
checkAllCheckBox = () => {
  const todos = [...this.state.todos]  // Better use const and initialize the array of objects directly
  // let newArray = [...todos]  same as in the first function,

  // isn't neccesary this if because if the array is empty, the map doesn't will iterate 
  // if (newArray.length !==0) {

  /* this is optional, but you can write this like
    const modifiedTodos = [...todos].map(({checked}) => checked = !checked)
  */

  /* In general, is better use const when possible because in this way 
  you will reassign a variable just when is necessary, and this is related with
  avoid mutate values. */
  const modifiedTodos = todos.map(item => {
    if (item.checked === true) {
      return item.checked = false
    } else {
      return item.checked = true
    }
  })

  this.checkBoxRouteUpdate()
  this.setState({ todos: modifiedTodos })
}

// Checking single Check Box

checkSingleCheckBox = (id) => {
  // since you need be secure that the todos is an array, you can do this instead of the destructuring
  const todos = [...this.state.todos] 
  // same as in the above function
  // let newArray = [...todos]

  // Here is better to use destructuring to get the _id and checked
  [...todos].forEach(({checked, _id}) => {
    /* this is totally personal preference but I try to avoid put a lot of code inside an if,
    to do this, you can do something like:

    if(_id !== id) return

    and your code doesn't need to be inside the if
    */
    if (_id === id) {
      /* this mutation is a little difficult to follow in large codebase, so, 
      is better if you modified the value in the place you will use it*/
      // checked = !item.checked 

      axios
        .put(`http://localhost:8080/edit/${id}`, {
          checked: !checked
        })
        .then(res => {
          this.setState({ todos: todos }) // or just {todos} if you use the object shorthand notation
          console.log('res', res)
        })
        .catch((err) => {
          console.log("err", err);
        });
    }
    // this else isn't necessary 
    // else {
    // }
  })

}

// Deleting Only Checked Items

deleteAllChecked = () => {
  const todos = this.state.todos.filter((item => item.checked !== true))
  /* Another way to do the above filtering is:
    const todos = this.state.todos.filter((item => !item.checked))
  */
  axios
    .delete('http://localhost:8080/deleteAllChecked')
    .then((res) => {
      this.setState({
        todos,
        pageCount: Math.ceil(todos.length / 10)
      })
      console.log("res", res);
    })
    .catch((err) => {
      console.log("err", err);
    });
}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.