javascriptclass

Updating JavaScript class instance


When updating properties on a class instance, is this the best approach? I could use some clarification from a more experienced developer on this.

Here is my class:

class Cart {
  constructor(id) {
    this.id = id;
    this.cartItems = [];
    this.cartSubtotal = 0;
    this.cartQuantity = 0;
  }

  getCartQuantity() {
    const qty =
      this.cartItems.reduce((quantity, item) => item.quantity + quantity, 0) ||
      0;
    return qty;
  }

  getCartSubTotal() {
    let subTotal = 0;
    this.cartItems.forEach((cartItem) => {
      const product = products.find(
        (product) => product.id === cartItem.productId
      );
      subTotal += cartItem.quantity * (product.price || 0);
    });
    return subTotal;
  }

  addItem(cartItem) {
    this.cartItems.push(cartItem);
  }
}

Here is my add cart item end point:

app.post('/addToCart/:cartId', (req, res) => {
  const { cartId } = req.params;
  const { id, quantity, detail } = req.body;
  const cart = getUserCart(cartId);

  const item = new CartItem(uuidv4(), id, quantity, detail);
  cart.addItem(item);
  cart.cartSubtotal = cart.getCartSubTotal();
  cart.cartQuantity = cart.getCartQuantity();

  res.json(cart);
});

Setting the values on the post call makes sense and it works but I feel like there is a more elegant way to do this.

cart.cartSubtotal = cart.getCartSubTotal();
cart.cartQuantity = cart.getCartQuantity();

Alternatively, would it make more sense to update the total and quantity from within the addToCart method instead of setting them in the post call?

addItem(cartItem) {
   this.cartItems.push(cartItem);
   this.cartSubtotal = this.getCartSubTotal();
   this.cartQuantity = this.getCartQuantity();
}

Any advice would be appreciated, thank you.


Solution

  • Would it make more sense to update the total and quantity from within the addToCart method instead of setting them in the post call?

    Yes, absolutely. This is the OOP best practice - you interact with an object only through method calls such as addToCart(), and the object itself is responsible for keeping its state up-to-date and consistent. You wouldn't want someone to update the cartItems but not the cartSubtotal or cartQuantity or only one of them. Also, if you were to introduce another cart property (e.g. average price), you would need to search for every call to addToCart() in the application and add another line to update the new property. That is precisely what modularisation is trying to avoid.

    So you'd use

    class Cart {
      constructor(id) {
        this.id = id;
        this.items = [];
        this.#update();
      }
      #update() {
        this.quantity = this.items.reduce((quantity, item) => item.quantity + quantity, 0);
        this.subTotal = this.items.reduce((subTotal, item) => item.quantity * (item.getProduct().price ?? 0) + subTotal, 0);
      }
      addItem(cartItem) {
        this.items.push(cartItem);
        this.#update();
      }
    }
    

    An alternative would be to not store the subTotal and quantity in the cart at all (which means having to update it every time the items change), but to only compute it when the property is accessed - using a get… method or a getter property:

    class Cart {
      constructor(id) {
        this.id = id;
        this.items = [];
      }
      get quantity() {
        return this.items.reduce((quantity, item) => item.quantity + quantity, 0);
      }
      get subTotal() {
        return this.items.reduce((subTotal, item) => item.quantity * (item.getProduct().price ?? 0) + subTotal, 0);
      }
      addItem(cartItem) {
        this.items.push(cartItem);
      }
    }