Salah Eddine Makdour
Salah Eddine Makdour

Reputation: 1042

How to fix the output 'undefined' when searching an array in Javascript

I was trying to make a javascript code that searches inside an objects array for text and then deletes the whole index (the index that includes the text property) but the code failed and always return 'undefined' so i wanted to get some help.

const todos = [{
    text: 'wake up',
    completed: true
}, {
    text: 'get some food',
    completed: false
}, {
    text: 'play csgo',
    completed: false
}, {
    text: 'play minecraft',
    completed: true
}, {
    text: 'learn javascript',
    completed: false
}]



let todo = function (todo, todoText) {
    return todo.find(function (text, index) {
        if (todo.text.toLowerCase() === todoText.toLowerCase()) {
            todo.splice(index, 1);
        }
    })
}

let deleteTodo = todo(todos, 'wake up');
console.log(deleteTodo);

i was expecting this output:

[{
    text: 'get some food',
    completed: false
}, {
    text: 'play csgo',
    completed: false
}, {
    text: 'play minecraft',
    completed: true
}, {
    text: 'learn javascript',
    completed: false
}]

but the output was actually 'undefined'

Upvotes: 0

Views: 3426

Answers (5)

Barmar
Barmar

Reputation: 781004

.find() requires the function to return a truthy value when the condition is matched. Your function doesn't return anything, it just splices the element out, so find() never returns the matching element.

You just need to add return true; after splicing the element.

let todo = function(todo, todoText) {
  todoText = todoText.toLowerCase();
  return todo.find(function(text, index) {
    if (text.text.toLowerCase() === todoText) {
      todo.splice(index, 1);
      return true;
    } else {
      return false;
    }
  })
}

const todos = [{
    text: 'wake up',
    completed: true
}, {
    text: 'get some food',
    completed: false
}, {
    text: 'play csgo',
    completed: false
}, {
    text: 'play minecraft',
    completed: true
}, {
    text: 'learn javascript',
    completed: false
}];

let deleteTodo = todo(todos, 'wake up');
console.log("Object that was removed:", deleteTodo);
console.log("Array that remains:", todos);

Earlier I wrote that you shouldn't modify the array while searching. But the specification of Array.prototype.find allows this, because it extracts the element from the array before calling the test function. Since you're only modifying the array when the match is found, it won't impact the rest of the iteration.

Upvotes: 2

StackSlave
StackSlave

Reputation: 10627

Not that I would use .find because of backward compatibility, but if you insist:

const todos = [{
    text: 'wake up',
    completed: true
}, {
    text: 'get some food',
    completed: false
}, {
    text: 'play csgo',
    completed: false
}, {
    text: 'play minecraft',
    completed: true
}, {
    text: 'learn javascript',
    completed: false
}];
let deleteTodo = function(todos, todoText){
  var e = todos.find(function(text){
    if(text.text.toLowerCase() === todoText.toLowerCase()){
      return true;
    }
  });
  return todos.splice(todos.indexOf(e), 1);
}
console.log(deleteTodo(todos, 'wake up')); // deleted
console.log(todos); // altered array

Here is a backward compatible version:

var todos = [{
    text: 'wake up',
    completed: true
}, {
    text: 'get some food',
    completed: false
}, {
    text: 'play csgo',
    completed: false
}, {
    text: 'play minecraft',
    completed: true
}, {
    text: 'learn javascript',
    completed: false
}];
function TodoWorker(todos){
  this.todos = todos;
  this.remove = function(text){
    for(var i=0,a=this.todos,l=a.length; i<l; i++){
      if(a[i].text === text){
        return a.splice(i, 1);
      }
    }
  }
}
var tw = new TodoWorker(todos);
console.log(tw.remove('play csgo')); // deleted element
console.log(tw.todos); // altered array
console.log(tw.remove('play minecraft')); // deleted element
console.log(tw.todos); // altered array - what you should be doing

You can see the second approach is both backward compatible and allows you to leave out your todos argument in it's .remove method.

Upvotes: 0

Le&#243;n Silva
Le&#243;n Silva

Reputation: 408

I think there's a better function to solve it: filter

let res = todos.filter( elem => elem.text.toLowerCase() !== 'wake up');

And if you want it to be a function, it would something like:

let result = (todos, todoText) => {
  return todos.filter( elem => elem.text.toLowerCase() !== todoText.toLowerCase() );
}
console.log(result(todos, 'wake up'));

Upvotes: 1

Mata
Mata

Reputation: 473

I think your code can work with a little modification:

const todos = [{
    text: 'wake up',
    completed: true
}, {
    text: 'get some food',
    completed: false
}, {
    text: 'play csgo',
    completed: false
}, {
    text: 'play minecraft',
    completed: true
}, {
    text: 'learn javascript',
    completed: false
}]



 function todo(todo, todoText) {
     let result = [];
     for (let index = 0; index < todo.length; index++) {
      
        if (todoText.toLowerCase() === todo[index].text.toString().toLocaleLowerCase()) {
        console.log("the deleted todo :",todo[index].text.toString().toLocaleLowerCase())
    }else{
            result.push(todo[index])
        }

     }
     return result;
}

let deleteTodo = todo(todos, 'wake up');
console.log(deleteTodo);

I've changed the loop a little and the function, using another variable for storing the result. Hope it helps

Upvotes: 0

Phil
Phil

Reputation: 164776

In the interest of immutability, I would instead filter the array to produce a new one without the matching records, ie

return todo.filter(({text}) => todoText.localeCompare(text, undefined, {
  sensitivity: 'base'
}))

String.prototype.localeCompare() will return a 0 value (falsy) if the strings match (ignoring case) or -1 / +1 (truthy) if they do not. This can be used to let Array.prototype.filter() know whether or not to include the entry in the final array.

const todos = [{
    text: 'wake up',
    completed: true
}, {
    text: 'get some food',
    completed: false
}, {
    text: 'play csgo',
    completed: false
}, {
    text: 'play minecraft',
    completed: true
}, {
    text: 'learn javascript',
    completed: false
}]

let todo = (todos, todoText) =>
  todos.filter(({ text }) => text.localeCompare(todoText, undefined, {
    sensitivity: 'base'
  }))

let deleteTodo = todo(todos, 'WAKE UP');
console.log(deleteTodo);

Upvotes: 1

Related Questions