17

Consider the following data:

food = {
  id: 1,
  name: 'Pizza',
  price: 16
};

orders = [
  { food_id: 2, table_id: 5 },
  { food_id: 2, table_id: 5 },
  { food_id: 1, table_id: 5 },
  { food_id: 3, table_id: 5 },
  { food_id: 1, table_id: 5 }
];

I want to remove a single item from the orders array matching food_id. Here's what I tried:

removeFoodOrder(food: Food): void {
  for (let order of this.orders) {
    let match = this.orders.filter((order) => order.food_id == food.id);
    match ? this.orders.splice(this.orders.indexOf(order), 1) : null;
    break;
  }
  console.log(this.orders);
}

If I call removeFoodOrder(food), it removes the first element from the array no matter what food item I pass in the params. 

removeFoodOrder(food) 
// removes {food_id: 2, table_id: 5} (the first element)
// I want to remove {food_id: 1, table_id: 5},

I want to target the matching element from the array and remove a single instance of it. Where did I go wrong?

Improve this question
3
  • It's removing the first element of the array because you've already removed the item with filter (I'm saying this without actually testing your code but I'm pretty sure that's what it is) Commented Jan 26, 2017 at 3:19
  • 2
    You have an unconditional break inside of your loop and you're using the ternary operator in an extremely suspicious way Commented Jan 26, 2017 at 3:25
  • 1
    @AluanHaddad I totally agree. That's just adding confusion Commented Jan 26, 2017 at 3:26

3 Answers 3

Reset to default
44

You could use Array#filter method:

food = {
  id: 1,
  name: 'Pizza',
  price: 16
};

orders = [
  { food_id: 2, table_id: 5 },
  { food_id: 2, table_id: 5 },
  { food_id: 1, table_id: 5 },
  { food_id: 3, table_id: 5 },
  { food_id: 1, table_id: 5 }
];

removeFoodOrder(food: Food): void {
  this.orders = this.orders.filter(({ food_id }) => food_id !== food.id);        
}

Edit:

Since your array allows duplicate elements and you want to remove only the first match, you could use the Array#findIndex + Array#filter methods:

const foundIndex = this.orders.findIndex(({ food_id }) => food_id === food.id);
this.orders = this.orders.filter((_, index) => index !== foundIndex);
Improve this answer
Sign up to request clarification or add additional context in comments.

8 Comments

Filter weeds out all matching elements. I only wish to remove one matching element.
Well, since the food_id isn't unique, how will you decide which element should be removed? The first.. the second.. the third? Think about it.
The first, of course. There can be many {food_id: 2, table_id: 5} duplicates in the orders array and I want to remove only one. Is it possible?
Well, filter method removes all elements that matches with a specific condition. Since your array allows duplicates, check the edit version with an alternative to tradittional for loop.
Thanks you so much! This is exactly what I was looking for.
|
3

The first step for me is always to remove anything confusing like that ternary operator and your break stmt. Here's how I did it

let food = {
  id: 1,
  name: 'Pizza',
  price: 16
}

let orders = [
    {food_id: 2, table_id: 5},
    {food_id: 2, table_id: 5},
    {food_id: 1, table_id: 5},
    {food_id: 3, table_id: 5},
    {food_id: 1, table_id: 5}
]

for (let order of this.orders) {
    if (food.id === order.food_id) {
        this.orders.splice(this.orders.indexOf(order), 1);
        break;
    }
}
console.log(this.orders);

I would recommend against using Array#filter if you don't know 100% how to use it.

UPDATE I am not saying don't use the Array#filter method. I'm just saying that if your code isn't working, you should try to remove anything that could be causing your issue and try to go about it step by step using simple constructs (like a for loop and if stmt).

Improve this answer

8 Comments

It's not a question of whether you should use filter or not is a question of what you're trying to do. If you want to create new array without the item in it filter is the right choice
@AluanHaddad You're absolutely right. I guess what I meant to say was the fastest way to solve a problem like this is to eliminate anything that one doesn't understand completely and try to fix it using basic constructs.
This answer is correct and works and is much better than the other answer but I don't think you should steer people away from filter because it's an extremely elegant and useful function. It may not be the right choice here though. Just remarking.
It depends if the element is unique or not. If it's not, then no (unless you do unholy things to it). If it is, then you would just use filter as expected.
I just noticed a big issue with this answer and suggested an edit. This answer is less well typed than the original problem. Get rid of the anys
|
1

Try this:

function removeFoodOrder(food: Food): void
{
    for (let order of this.orders) {
        if (order.food_id == food.id) {
            this.orders.splice(this.orders.indexOf(order), 1);
            break;
        }      
    }
    console.log(this.orders);
};
Improve this answer

2 Comments

@AluanHaddad can you please elaborate on poor style?
The use of == as well as the addition of irrelevant abstractions and unnecessary type annotations.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.