Chimoneg27 / To-do-list-review

This is for a peer to peer feedback exercise
MIT License
2 stars 0 forks source link

Peer to peer code review #2

Open zhorabay opened 1 year ago

lucy-sees commented 1 year ago

Hi @Chimoneg27 ,

Your project is looking great!

To Highlights

:heavy_check_mark: Descriptive commit messages :heavy_check_mark: Professional** looking README file :heavy_check_mark: Good overall design :heavy_check_mark: Good use of GitHub flows 👍 :heavy_check_mark: Overall, the code seems well-organized and follows JavaScript best practices. It separates concerns, uses functions for modularization, and efficiently updates the UI and local storage when interacting with todos.

Suggestion

There's a potential issue with reassigning the todos array in the clearCompletedBtn event listener. clearCompletedBtn.addEventListener('click', () => { todos = todos.filter((todo) => !todo.completed);

todos.forEach((todo, index) => { todo.id = index; });

renderTodos(); localStorage.setItem('todos', JSON.stringify(todos)); }); It is not good practice to reassign a variable declared with let outside its scope. Instead, use methods like splice or create a new array with filtered values. The same goes for the event listener that handles checkbox changes.

Cheers and Happy coding!👏👏👏

zhorabay commented 1 year ago

Hi @Chimoneg27 ,

Your project is looking great!

To Highlights ✔️ Descriptive commit messages ✔️ Professional** looking README file ✔️ Good overall design ✔️ Good use of GitHub flows 👍 ✔️ Overall, the code seems well-organized and follows JavaScript best practices. It separates concerns, uses functions for modularization, and efficiently updates the UI and local storage when interacting with todos.

Suggestion

renderTodos(); localStorage.setItem('todos', JSON.stringify(todos)); });

Cheers and Happy coding!👏👏👏