Function only removes items from array but does not change values

Question

I have created a class that takes computer parts and their costs and calculates them based on what parts are chosen. At the moment I have two functions, one to add more parts to the quote and one to remove parts. It works correctly in the sense of removing or adding items, but does not change the total cost. When I remove parts, the price remains the same, likewise if I add more parts. What could I do to get this working as expected. Here is the code:

class PriceCalc {
  Motherboard = 520.99;
  RAM = 250.4;
  SSD = 500.8;
  HDD = 400.66;
  Case = 375.5;
  Monitor = 600.75;
  Keyboard = 100.99;
  Mouse = 25.5;

  constructor(Obj) {
    this.parts = Obj;
    this.cost = "$" + Obj.reduce((a, b) => a + this[b], 0).toFixed(2);
    this.retail ="$" +(Obj.reduce((a, b) => a + this[b], 0) +Obj.reduce((a, b) => a + this[b], 0) * 1.75).toFixed(2);
    this.quote = "Your quote is " + this.retail;
  }
  add(item) {
    this.parts = [...this.parts, item];
  }
  remove(item) {
    this.parts = this.parts.filter((x) => x !== item);    
  }
}

quote4 = new PriceCalc(["RAM", "SSD", "Case", "Mouse"]);
console.log(quote4.parts);//Returns ["RAM", "SSD", "Case", "Mouse"]
console.log(quote4.cost);//Returns $1152.20
console.log(quote4.retail);//Returns $3168.55
console.log(quote4.quote);//Returns "Your quote is $3168.55"

quote4.remove("Case")
console.log(quote4.parts);//Returns ["RAM", "SSD", "Mouse"]
console.log(quote4.cost);//Returns $1152.20
console.log(quote4.retail);//Returns $3168.55
console.log(quote4.quote);//Returns "Your quote is $3168.55"

At the moment this.cost/retail/quote doesnt change when things are added or removed, whereas they should be modified if items are added or removed. The only way I can change the values at the moment is by manually changing the parts within the called array. How could I fix this?

You need to recalculate them whenever a new item gets added or an item is removed. Inside both `add` and `remove` you should do `this.cost = ...` and `this.retail = ...` and `this.quote = ...` or better yet, group those in a new method called, for example, `recalculate` and call it instead (call it from the `constructor` too). Reusable code should be grouped in a function. — ibrahim mahrir, Jun 21 '20 at 23:55
I tried calling methods again within each of the functions, but since Obj is called within the constructor, It doesn't recognise it outside that scope — dwewers, Jun 21 '20 at 23:59

ibrahim mahrir · Answer 1 · 2020-06-22T00:27:00.360

You need to recalculate them whenever a new item gets added or an item is removed. Since they need to be recalculated from different places (the constructor, add and remove), a new method (called calculate or update for example) is perfect for this as reusable code should be grouped in a function/method so you don't repeat yourself.

Also, cost and retail should be numbers instead of strings. You are using reduce to calculate cost, then using the same reduce twice to calculate retail which should be avoided. Just calculate cost and use cost to calculate retail.

Finally, add and remove should not create new arrays each time an item is removed or added.

class PriceCalc {
  Motherboard = 520.99;
  RAM = 250.4;
  SSD = 500.8;
  HDD = 400.66;
  Case = 375.5;
  Monitor = 600.75;
  Keyboard = 100.99;
  Mouse = 25.5;

  constructor(initialParts) {                                    // always choose good names for your variables. initialParts makes more sense than Obj
    this.parts = initialParts;                                   // store the initial parts
    calculate();                                                 // calculate cost, retail and quote
  }

  add(item) {
    this.parts.push(item);                                       // use push instead of creating a new array (which is an overkill)
    calculate();                                                 // recalculate cost, retail and quote
  }

  remove(item) {
    this.parts = this.parts.filter((x) => x !== item);           // I'd rather use this https://stackoverflow.com/a/5767357 over filter but filter is shorter so OK
    calculate();                                                 // recalculate cost, retail and quote
  }

  calculate() {                                                  // the method that calculates cost, retail and quote
    this.cost = this.parts.reduce((a, b) => a + this[b], 0);     // calculate cost and store as a number instead of a string
    this.retail = this.cost + this.cost * 1.75;                  // calculate retail using the value of this.cost and also store as a number (you can use this.retail = 2.75 * this.cost; which is the same)
    this.quote = "Your quote is $" + this.retail.toFixed(2);     // generate the quote (notice the added $ sign and the call to toFixed(2))
  }
}

Function only removes items from array but does not change values

1 Answers1