Why does the filter method not seem to work the same way as splice does in this todo app?-CodePudding

I have a todo app in JS with the following functions:

This is part of a function that passes an id into an event listener to remove a todo

removeButton.addEventListener('click', function () {
                removeTodo(todo.id)
                renderTodos(todos, filters)
            })

This function removes the todo - I've used 2 approaches, the findIndex way works great, it removes the todo and renders the new todos fine - I thought the filter approach I've commented would also work but it doesn't, it does remove the todo but it doesn't automatically update the list in the browser unless I refresh the page, while splice does it automatically, why could this happen? could it be waiting to update local storage before renderTodos starts reading the list? Just a note that in the example that didn't work I was passing newTodos into the save function, I just changed it to todos for the splice way.

const removeTodo = function (id) {
const todoIndex = todos.findIndex(function (todo) {
    return todo.id === id
})

if (todoIndex > -1) {
    todos.splice(todoIndex, 1)
}
// newTodos = todos.filter(function (todo) {
//     return todo.id !== id
// })
saveTodos(todos)
}

the todo list is saved in local storage

const saveTodos = function (todos) {
    localStorage.setItem('todos', JSON.stringify(todos))
}

Here is the render function for information

const renderTodos = function (todos, filters) {
        const filteredTodos = todos.filter(function (todo) {
            const searchTextMatch = todo.text.toLowerCase().includes(filters.searchText)
            const hideCompletedMatch = !filters.hideCompleted || !todo.completed
            return searchTextMatch && hideCompletedMatch
        })
            
        const todosLeft = filteredTodos.filter(function (todo) {
            return !todo.completed
        })
        document.querySelector('#todos').innerHTML = ''
        document.querySelector('#todos').appendChild(generateSummaryDom(todosLeft))

        filteredTodos.forEach(function (todo) {
            document.querySelector('#todos').appendChild(generateTodoDom(todo))
        })
}

CodePudding user response：

Reassigning a variable does not have side-effects; reassigning one identifier has no effect on identifiers elsewhere. Doing

  newTodos = todos.filter(function (todo) {
    return todo.id !== id
  })
  saveTodos(todos)
}

means that you've put some results into newTodos without doing anything else with it. It doesn't get put into storage (or rendered, though how you render isn't shown).

Pass along the new filtered todos, and render (however you're doing it) from there - and don't forget to declare your variables.

  const newTodos = todos.filter(function (todo) {
    return todo.id !== id
  })
  saveTodos(newTodos);
  renderTodos(newTodos);

while taking renderTodos out of the immediate listener callback.

CodePudding user response：

splice() mutates the todos array which you are then renderering, while filter() returns a new array which you are not making use of.

To make it work with filter() you will need to return the newTodos from the remove function and render the returned array, not the original todos array.

removeButton.addEventListener('click', function () {
  const newTodos = removeTodo(todo.id);
  saveTodos(newTodos)
  renderTodos(newTodos, filters);
})

const removeTodo = function (id) {
  return todos.filter(todo => todo.id !== id)
}

const saveTodos = function (todos) {
  localStorage.setItem('todos', JSON.stringify(todos))
}