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

Question

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'

You are returning the return value from Array.prototype.find() but your callback doesn't return anything so find() finds nothing (ie undefined) — Phil
– Phil, Commented Aug 23, 2019 at 0:59
Don't use splice just return value from find callback, or use filter to remove the unwanted values — Code Maniac
– Code Maniac, Commented Aug 23, 2019 at 1:05

Barmar · Accepted Answer · 2019-08-23 19:11:13Z

2

.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.

edited Aug 23, 2019 at 19:11

answered Aug 23, 2019 at 1:09

Barmar

789k57 gold badges555 silver badges669 bronze badges

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

3 Comments

Salah Eddine Makdour Over a year ago

Thanks for the answer, but when i ran your code it output this ` { 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 } ] ` but i want the object with the text: 'wake up' to be completely removed from the array.

Barmar Over a year ago

It is removed from the array. There are two console.log() statements. One is showing the object that was removed, the other is showing the array with the object removed.

Salah Eddine Makdour Over a year ago

i really didnt notice omg thanks man for the help it actually works

Phil · Accepted Answer · 2019-08-23 01:16:03Z

1

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);

edited Aug 23, 2019 at 1:16

answered Aug 23, 2019 at 1:05

Phil

166k25 gold badges265 silver badges269 bronze badges

8 Comments

Code Maniac Over a year ago

What i am understanding from code is op is trying to remove matching todos, i think negation should be used here

Phil Over a year ago

@CodeManiac I'm cheating somewhat by using the implicit Boolean inversion of String.prototype.localeCompare(). If the strings are equal, it returns 0 which will filter out that value

Barmar Over a year ago

He wants the function to return the matching todo, and also remove it from the list. This just returns the new list, but not the matching todo.

Phil Over a year ago

@Barmar that's not clear from the question or the code in the question so I made an assumption

Barmar Over a year ago

His use of find() is strongly suggestive of trying to return the element.

|

León Silva · Accepted Answer · 2019-08-23 01:30:10Z

1

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'));

edited Aug 23, 2019 at 1:30

answered Aug 23, 2019 at 1:22

León Silva

4084 silver badges7 bronze badges

1 Comment

Phil Over a year ago

Note: OP seems to want to compare the text without case sensitivity

Mata · Accepted Answer · 2019-08-23 01:25:23Z

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

StackSlave · Accepted Answer · 2019-08-23 01:52:43Z

0

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.

edited Aug 23, 2019 at 1:52

answered Aug 23, 2019 at 1:22

StackSlave

10.6k2 gold badges20 silver badges35 bronze badges

4 Comments

Barmar Over a year ago

find() doesn't return the index, it returns the element.

StackSlave Over a year ago

Thanks, I haven't used .find that often. I'm surprised I got the expected result. I fixed it, though. I will supply a backward compatible version as well. Hold on... I need a sip of coffee.

Barmar Over a year ago

It was only working by accident, because the matching element was the first element in the array. You had return index when it matched, and index was 0, which is falsey. So find() didn't find a match and returned undefined. Then when you called todos.splice(i, 1), the undefined value of i was converted to 0 by splice().

StackSlave Over a year ago

Thanks @Barmar, I realized that and gave you points for your first comment. Of course... someone had to vote you up again. The rich get richer.

Collectives™ on Stack Overflow

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

5 Answers 5

3 Comments

8 Comments

1 Comment

Comments

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

3 Comments

8 Comments

1 Comment

Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related