London | 26-ITP-January | Eugenie Ahangama | Sprint 3 | Todo List App

[Eugenie-A]

Learners, PR Template

Self checklist

• I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
• My changes meet the requirements of the task
• I have tested my changes
• My changes follow the style guide

Changelist

• Added the "Delete completed tasks" button in index.html
• Implemented deleteCompleted() in todos.mjs
• Wrote Jest tests for deleteCompleted() in todos.test.mjs
• Added event listener for delete completed button in script.mjs
• Implemented deleteCompletedTodos callback in script.mjs

Questions

Why is render without parentheses a bug?

[kyle-tightest]

this is not case sensitive so it doesn't matter but it is more standard to capitalise this.
did your formatter do this automatically?

[Eugenie-A]

Yes, that was my Prettier formatter in VS Code doing it automatically on save. I know it's more standard to keep it uppercase but Prettier lowercases it by default. Prettier formatter is the standard formatter we were asked to use in our work.

[kyle-tightest]

Okay no problem

[kyle-tightest]

While this code works, it is inefficient as you are iterating over the entire array when you do filter and then iterating over each of the filtered array items when you do forEach.
The .filter method returns an array filtered by the condition you give it.
Try and see if you can use the .filter method differently to achieve the same outcome, without needing forEach?

[Eugenie-A]

Thank you for the feedback. I've updated the function to use filter once to get the incomplete todos, then splice to update the original array in place.

[kyle-tightest]

Thanks, this works as well.
But again, when you do splice in that second step you're iterating over each of the filtered array items.
This is not necessary.
Try to look at the function again and see if you can do it in 1 line, instead of 2?

[Eugenie-A]

I've combined both lines into one, the filter now happens directly inside the splice call so it's done in a single step.

[kyle-tightest]

Almost there! you can remove the splice and just use filter

[Eugenie-A]

If I just use filter and return the result, won't the tests fail since they check the original todos array after calling the function?

[kyle-tightest]

Ah sorry you're 100% correct, the function is built to change the array in place instead of returning a new array as a result. Okay nevermind then this is perfect

[illicitonion]

Closing PR because the January ITP run has finished. Feel free to re-open if you're still working on it.