Every time when a product is added to shoppingBasket if its already in map basket it should increase it value amount by 1. But it doesn't for some reason. Is it because every time im adding a product to map i'm creating a new purchases? I can't figure it out.
public void add(String product, int price) {
Purchases buy = new Purchases(product, 1, price);
if(!basket.containsKey(product)) {
basket.put(product, buy);
} else {
buy.increaseAmount();
}
/
public void increaseAmount() {
this.amount = 1;
}
/
public class Main {
public static void main(String[] args) {
ShoppingBasket basket = new ShoppingBasket();
basket.add("milk", 3);
basket.print();
System.out.println("basket price: " basket.price() "\n");
basket.add("buttermilk", 2);
basket.print();
System.out.println("basket price: " basket.price() "\n");
basket.add("milk", 3);
basket.print();
System.out.println("basket price: " basket.price() "\n");
basket.add("milk", 3);
basket.print();
System.out.println("basket price: " basket.price() "\n");
}
}
/
import java.util.HashMap;
import java.util.Map;
public class ShoppingBasket {
private Map<String,Purchases> basket;
public ShoppingBasket() {
this.basket = new HashMap<String,Purchases>();
}
public void add(String product, int price) {
Purchases buy = new Purchases(product, 1, price);
if(!basket.containsKey(product)) {
basket.put(product, buy);
} else {
buy.increaseAmount();
}
}
public int price() {
int price = 0;
for(Purchases item : basket.values()) {
price = item.price();
}
return price;
}
public void print() {
Map<String, Integer> test = new HashMap<String,Integer>();
for(Purchases item : basket.values()) {
test.put(item.product(), item.amount());
}
for(String key : test.keySet()) {
Integer value = test.get(key);
String complete = key ": " value;
System.out.println(complete);
}
}
}
/
public class Purchases {
private String product;
private int amount;
private int unitPrice;
public Purchases(String product,int amount, int unitPrice) {
this.product = product;
this.amount = amount;
this.unitPrice = unitPrice;
}
public int price() {
return this.amount * this.unitPrice;
}
public void increaseAmount() {
this.amount = 1;
}
public String toString() {
return "" this.amount;
}
public int amount() {
return this.amount;
}
public String product() {
return this.product;
}
}
CodePudding user response：
In your else black, you need to retrieve the Purchase object from the map. Then call increaseAmount on the object retrieved.
public void add(String product, int price) {
Purchases buy = new Purchases(product, 1, price);
if(!basket.containsKey(product)) {
basket.put(product, buy);
} else {
buy = basket.get(product); <--retrieve it
buy.increaseAmount(); <--increment amount
}
}
CodePudding user response：
code is not adding purchase to map if key is already there, add map.put in else part
public void add(String product, int price) {
Purchases buy = new Purchases(product, 1, price);
if(!basket.containsKey(product)) {
basket.put(product, buy);
} else {
buy.increaseAmount();
basket.put(product, buy);
}
}
CodePudding user response：
Purchases buy = new Purchases(product, 1, price);
See the new? You're making a new object here, hence the name. You then increment the product count on this new object and promptly toss the only variable that refers to this new object (your buy variable) in the garbage, because that's what happens with all local variables when a method ends: The variable goes away. With that, nothing refers to this brand new Purchases instance and thus it'll eventually be garbage collected.
You want to query for the actual object that was made earlier and stored in that map, then increment the product count on that.
In your code, you make a new Purchases instance no matter what happens, and then map the given string to this newly created purchases object only if the string isn't already in your map. This is no good. You want to create a new purchases instance only if it isn't already in the map, otherwise you want to fetch the existing instance of Purchases.
You could do this:
Purchases buy;
if(!basket.containsKey(product)) {
basket.put(product, buy = new Purchases(product, 1, price));
} else {
basket.get(product).increaseAmount();
}
But this is inefficient, 'ugly' (kinda hard to maintain), and outright broken if this is a concurrent hashmap. Much better is to act-then-check instead:
basket
.computeIfAbsent(product, k -> new Purchases(product, 0, price))
.increaseAmount();
This code does just what it says: It will compute the requisite value, but only if there is no key/value mapping yet. So, IF product is in the map, you just get the Purchases instances associated with it and we move immediately on to .increaseAmount(). But if it is not, the code new Purchases(product, 0, price) is executed and whatever that resolves to is used as the value (So, it's like .put(product, new Purchases(...)), except the new part is only run if that item wasn't in the map already. There's a k -> in there because [A] it's a closure, it's code that is sent to the computeIfAbsent method and is only actually run if needed, and [B] you get the key passed along. Here, not needed, you already have this (variable product), but you can imagine you invoked perhaps basket.computeIfAbsent(some.complex().calculation(), ...., that's why it's there.
You then call increaseAmount() on whatever you get. Hence why this code starts out with an amount of 0: Because it'll be increment to 1 immediately afterwards.
CodePudding user response：
This is because you increase the amount in the new Purchases object which is not in the map.
Let's do code analysis:
[Map State] EMPTY
You put one Purchases object with product's name/key "milk" inside the map:
public void add(String product, int price) {
Purchases buy = new Purchases(product, 1, price);
if(!basket.containsKey(product)) {
// Executed
basket.put(product, buy);
} else {
buy.increaseAmount();
}
}
As you probably know the code found in the if block is executed - Adds the object inside buy to the map.
[Map State] { "milk", PurchasesObject1 }
Now you are trying again to put a NEW Purchases object whose product's/key value is "milk" again into the map. As you probably know, the else block will be executed, Because Purchases object with key "milk" already exists in the map.
What happens in the else block is that you increase the value of amount of the NEW local object you've just created inside the method, which is not even in the map at all, so once the add() method finishes its execution, the object you've created becomes garbage that needs to be collected by the garbage collector.
Solution?
Sure. Just retrieve the object with the same key and do what you wanted on it.
public void add(String product, int price) {
Purchases buy = new Purchases(product, 1, price);
if(!basket.containsKey(product)) {
// Executed
basket.put(product, buy);
} else {
basket.get(product).increaseAmount();
}
}