Remove and add object item into array and change object key if specific item already exists in the array

Question

I have this array of recipes where each object is a specific recipe and each recipe has an array of ingredients and each ingredient it's an object made of _id name quantity. My problem added below if you guys have any idea why is this happening please let me know. I am struggling for 3 days...any advice would be really appreciated. Thanks a lot!

(3) [{…}, {…}, {…}]
0:
ingredients: Array(4)
0: {_id: "5f6628d0029e87e02c79ce0a", name: "chia", quantity: 10}
1: {_id: "5f6628d0029e87e02c79ce0b", name: "apple", quantity: 15}
2: {_id: "5f6628d0029e87e02c79ce0c", name: "honey", quantity: 30}
3: {_id: "5f6628d0029e87e02c79ce0d", name: "almond flour", quantity: 35}
length: 4
__proto__: Array(0)
name: "Coconut Chia Pudding"
__v: 0
_id: "5f6628d0029e87e02c79ce09"
__proto__: Object
1: {_id: "5f6628d0029e87e02c79ce0e", name: "Peanut Butter Cookies", ingredients: Array(4), __v: 0}
2: {_id: "5f6628d0029e87e02c79ce13", name: "Caprese Avocado Bowls", ingredients: Array(3), __v: 0}
length: 3
__proto__: Array(0)

What I have in UI it's a list with the above recipes which a user can tick and untick and after a recipe has been ticked its ingredients are showed into a list.

HTML

<ion-content>
  <ion-grid>
    <ion-row>
      <ion-col>
        <ion-list>
          <ion-item
            *ngFor="let recipe of loadedRecipes; let lastRecipe = last"
            [ngClass]="{ 'last-recipe': lastRecipe }"
          >
            <ion-checkbox
              (ionChange)="onCheckRecipe($event)"
              value="{{recipe.name}}"
            ></ion-checkbox>
            <ion-label>{{recipe.name}}</ion-label>
            <ion-button
              [routerLink]="['/','recipes','recipe-details', recipe._id]"
              >></ion-button
            >
          </ion-item>
        </ion-list>
      </ion-col>
    </ion-row>

    <ion-row>
      <ion-col>
        <h6 class="ion-padding-start" *ngIf="groceryList.length > 0">
          Grocery List
        </h6>
        <ion-list *ngIf="groceryList.length > 0">
          <ion-item *ngFor="let ingredient of groceryList">
            <ion-label>{{ingredient.name}}</ion-label>
            <ion-note slot="end">{{ingredient.quantity}} g</ion-note>
          </ion-item>
        </ion-list>
      </ion-col>
    </ion-row>
  </ion-grid>
</ion-content>

What I want to achieve (and I've done it below, but I have a bug) is when the user ticks a recipe its ingredients to be added into an array called groceryList and when unticks the recipe to remove the ingredients from my groceryList array. Also, if I have ticked 1 recipe and if the next one I am going to tick has the same ingredient as the one ticked before, just to increment that common ingredient quantity that already exists, and NOT to add it twice and if I want to untick a recipe to remove the uncommon ingredients and subtract the quantity of the common ingredient. I already managed to do it, BUT I have a big problem and I don't know where is coming from. at some point in UI if I tick and untick recipes and I tick the ones that have the same ingredient one after another it removes the common ingredient even though I still have a ticked recipe that has that ingredient. Again, if you guys have any idea why is this happening please let me know I would appreciate any advice you have

My TS

import { Component, OnInit } from "@angular/core";
import { Subscription } from "rxjs";
import { RecipesService } from "src/app/services/recipes.service";

@Component({
  selector: "app-recipes",
  templateUrl: "./recipes.page.html",
  styleUrls: ["./recipes.page.scss"],
})
export class RecipesPage implements OnInit {
  loadedRecipes: any;
  private _recipesSub: Subscription;

  constructor(private recipesService: RecipesService) {}
  groceryList = [];

  ngOnInit() {
    this._recipesSub = this.recipesService.recipes.subscribe((receivedData) => {
      this.loadedRecipes = receivedData;
    });
  }

  onCheckRecipe(e) {
    if (e.detail.checked === true) {
      for (let recipe of this.loadedRecipes) {
        console.log(this.loadedRecipes);
        if (recipe.name === e.detail.value) {
          for (let eachIngredient of recipe.ingredients) {
            let matchedIng = this.groceryList.find(function (foundIng) {
              return foundIng.name === eachIngredient.name;
            });
            if (matchedIng) {
              matchedIng.quantity =
                matchedIng.quantity + eachIngredient.quantity;
            } else {
              this.groceryList.push(eachIngredient);
            }
          }
        }
      }
    } else {
      for (let recipe of this.loadedRecipes) {
        if (recipe.name === e.detail.value) {
          for (let eachIngredient of recipe.ingredients) {
            let matched = this.groceryList.find(function (foundIngre) {
              return foundIngre.name === eachIngredient.name;
            });
            if (
              matched.name === eachIngredient.name &&
              matched._id === eachIngredient._id
            ) {
              let index = this.groceryList.findIndex(
                (x) => x._id === matched._id
              );
              this.groceryList.splice(index, 1);
            } else {
              matched.quantity = matched.quantity - eachIngredient.quantity;
            }
          }
        }
      }
    }
  }

  ionViewWillEnter() {
    this.recipesService.fetchRecipes().subscribe();
  }

  ngOnDestroy() {
    if (this._recipesSub) this._recipesSub.unsubscribe();
  }
}

Linda Paiste · Accepted Answer · 2020-09-24 22:17:13Z

1

The problem lies in the flow of your if statements. In the "onRemove" section of your code, you are saying "If the ingredient is in the list, remove it from the list. If not, decrement its quantity." That second part doesn't make any sense and more importantly, you'll never reach it because the ingredient should always be in the list.

for (let eachIngredient of recipe.ingredients) {
  let matched = this.groceryList.find(function(foundIngre) {
    return foundIngre.name === eachIngredient.name;
  });
  if (
    matched.name === eachIngredient.name &&
    matched._id === eachIngredient._id
  ) {
    let index = this.groceryList.findIndex(
      (x) => x._id === matched._id
    );
    // Problem e ca eachIngredient.quantity se schimba
    this.groceryList.splice(index, 1);
  } else {
    matched.quantity = matched.quantity - eachIngredient.quantity;
  }
}

Based on what you've said, what you want to do is:

subtract the quantity which was attributed to the removed recipe
if the new quantity is zero, remove the ingredient from the list (though you could also leave it and ignore ingredients with zero quantity)

Try this instead:

for (let eachIngredient of recipe.ingredients) {
  // I am assuming that ids are unique so I am not checking foundIngre.name at all, 
  // since I assume that ingredients with the same name must also have the same name
  // I am also using findIndex first so that you don't need a second find when removing
  const matchIndex = this.groceryList.findIndex( 
     (foundIngre) => foundIngre._id === eachIngredient._id
  );
  if ( matchIndex ) { // this should always be true
    const matched = this.groceryList[matchIndex];
    // preserve the entry if there is still some quantity
    if ( matched.quantity > eachIngredient.quantity ) {
      matched.quantity = matched.quantity - eachIngredient.quantity; // can use -= to shorten
    }
    // remove from the list only if there is no quantity remaining
    else {
        this.groceryList.splice(matchIndex, 1);
    }
  }
}

EDIT: Trying to update and remove items in an array is an unnecessary pain. The reworked version of your code stores the _groceryList in a keyed dictionary instead. I initially intended to key by ingredient id, but after reviewing your demo I see that I was incorrect in my assumption that the same ingredient in multiple recipes would share the same id. So instead I am keying by ingredient name. This way you can write to _groceryList[name] and it doesn't matter whether it previously existed or not.

The class has a public getter groceryList which converts the private _groceryList dictionary into an array.

I've also tried to remove unnecessary code duplication across scenario branches by using a generic toggleIngredient function which uses the boolean checked to control whether it is adding or subtracting by multiplying by plus or minus one.

import { Component } from "@angular/core";
import { Platform } from "@ionic/angular";
import { SplashScreen } from "@ionic-native/splash-screen/ngx";
import { StatusBar } from "@ionic-native/status-bar/ngx";
import { Subscription } from "rxjs";

export interface Ingredient {
  _id: string;
  name: string;
  quantity: number;
}

export interface Recipe {
  _id: string;
  name: string;
  ingredients: Ingredient[];
}

@Component({
  selector: "app-root",
  templateUrl: "app.component.html"
})
export class AppComponent {

  private _recipesSub: Subscription;
  constructor(
    private platform: Platform,
    private splashScreen: SplashScreen,
    private statusBar: StatusBar,
  ) {
    this.initializeApp();
  }

  initializeApp() {
    this.platform.ready().then(() => {
      this.statusBar.styleDefault();
      this.splashScreen.hide();
    });
  }
  private loadedRecipes: Recipe[] = [/*...*/]

  // store the groceryList in a dictionary keyed by name
  private _groceryList: Record<string, Ingredient> = {};

  // getter returns the groceryList in array format, ignoring 0 quantities
  get groceryList(): Ingredient[] {
    return Object.values(this._groceryList).filter( ing => ing.quantity > 0 );
  }

  // get the current quantity for an ingredient by name, or 0 if not listed
  currentQuantity( name: string ): number {
    const ingredient = this._groceryList[name];
    return ingredient ? ingredient.quantity : 0;
  }

  // update the quantity for an ingredient when checked or unchecked
  // will add new ingredients, but never removes old ones
  toggleIngredient( ingredient: Ingredient, checked: boolean ): void {
    // add to or remove from quantity depending on the value of checked
    const quantity = this.currentQuantity(ingredient.name) + (checked ? 1 : -1 ) * ingredient.quantity;
    // replace the object in the grocery list dictionary
    this._groceryList[ingredient.name] = {
      ...ingredient,
      quantity
    }
  }

  onCheckRecipe(e) { // you'll want to add a type for e here
    for (let recipe of this.loadedRecipes) {
      // find the matching recipe
        if (recipe.name === e.detail.value) {
          // loop through the recipe ingredients
          for (let eachIngredient of recipe.ingredients) {
            this.toggleIngredient(eachIngredient, e.detail.checked)
          }
        }
      }
  }
}

edited Sep 24, 2020 at 22:17

answered Sep 24, 2020 at 19:46

Linda Paiste

42.6k8 gold badges81 silver badges118 bronze badges

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

7 Comments

alex Over a year ago

Hi! Thank you for your reply! I just tried your option but when I get to a recipe which has the matching ingredient and I untick it I get in the console " Cannot read property 'quantity' of undefined" so I've tried something like this if (matchIndex !== -1) {...} and not errors, but now the behaviour that I get is, for example, I tick all the recipes, I untick one which has the common ingredient it keeps that common ingredient but it doesn't decrease the quantity. DO you have any idea why this might be?

Linda Paiste Over a year ago

Hmm I really don't know. It could be that this.groceryList[matchIndex] is accessing an invalid index, but I thought that was handled with if ( matchIndex ) {}. I'm going to play with the code a bit more. Breaking onCheckRecipe into a bunch of smaller pieces could help a lot with clarity. Can you share a link to your demo?

alex Over a year ago

Thanks a lot! This is what I'm trying to do as well.. here is my demo stackblitz.com/edit/ionic-v4-chnobn?file=src/app/… please let me know if it works for you

Linda Paiste Over a year ago

Ok here you go! stackblitz.com/edit/ionic-v4-heofbc?file=src/app/…

Linda Paiste Over a year ago

There's a lot of individual scenarios that have to be dealt with (combining checked true/false with ingredient is in array true/false). You want to prefer abstraction over branching through "if" statements when you can. In your original code you've ended up with a lot of repeated lines in your onChecked and onUnchecked situations because ultimately they are doing the same thing with only minor differences. You want to make that difference as small as possible so that all the rest of the code can be shared. In my version, that difference is reduced down to checked ? 1 : -1 , add vs. subtract.

|

RPDP · Accepted Answer · 2020-09-24 19:41:08Z

0

I think the issue seems to be with this section of the code

if (
  matched.name === eachIngredient.name &&
  matched._id === eachIngredient._id
) {
  let index = this.groceryList.findIndex(
    (x) => x._id === matched._id
  );
  // Problem e ca eachIngredient.quantity se schimba
  this.groceryList.splice(index, 1);
} else {
  matched.quantity = matched.quantity - eachIngredient.quantity;
}

The if statement should be checking on the quanity instead of validating the name and id again , something like

if(matched.quantity <= eachIngredient.quantity){ // splice the item and remove it. } else { // decrease the quantity }
A small suggestion to finding the matched ingredient. First use findIndex() to retrieve matchedIndex, and use grocerylist[matchedIndex] to retrieve the item, to avoid iterating through the grocerylist again to find the index for splicing.

answered Sep 24, 2020 at 19:41

RPDP

1,7231 gold badge11 silver badges6 bronze badges

1 Comment

alex Over a year ago

Hi! Thank you for your reply! I've tried your point 1 but unfortunately still the same behaviour and regarding 2 thank you so much for the advice, I'll make these changes accordingly

Collectives™ on Stack Overflow

Remove and add object item into array and change object key if specific item already exists in the array

2 Answers 2

7 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

7 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related