receipt is Map or Set?

Question

In my application I have custom Price type defined as follows

public final class Price {
    private BigDecimal cash;
    private Currency currency;
    ...
}

with default Object#equals and Object#hashCode methods, since it's not required (I never check Price objects logical equality).

Also, there is order receipt that represents the total price of goods in the order. If there are 3 items in order with prices 2 USD, 3 USD and 2 GBP, then receipt should contain the entities 5 USD and 2 GBP.

For now I represent receipt as map with java.util.Currency keys

Map<Currency, BigDecimal> receipt;

My question is if it will be a good design in changing it to

Set<Price> receipt;

and reuse my custom Price type with overridden Object#equals and Object#hashCode methods as

@Override
public boolean equals(Object o) {
    if (this == o) return true;
    if (o == null || getClass() != o.getClass()) return false;

    Price price = (Price) o;

    return currency.equals(price.currency);
}

@Override
public int hashCode() {
    return currency.hashCode();
}

so that

Price usd_2 = new Price(new BigDecimal(2), Currency.getInstance("USD"));
Price usd_3 = new Price(new BigDecimal(3), Currency.getInstance("USD"));
usd_2.equals(usd_3);    // true

Will it be a bad choice to override equality methods in this way and what possible pitfalls it may cause in the future?

Answer 1

There are a few design improvements you could make, which I'm sure will pop up in comments, but I'm going to ignore those and focus on your specific questions.

So, in a comment, you say a "receipt" is:

It's a receipt for goods to be paid for. If Customer purchases 2 Items with prices 2 USD and 3 USD, then receipt contains 5 USD.

It represents total cost of the items, it does not track individual items.

Goods that refer to the same Receipt could be of different Currency, that's why I use Map. F.e, receipt may contain 5 USD and 4 GBP.

So as for your question about changing it from a Map to a Set: Leave it a Map. This makes sense. Your receipt is a collection of totals in various currencies, one per currency. You could change it to a Set<Price> but then, as you've discovered, you have to use weird, counterintuitive implementations of equals and hashCode, and a Set doesn't realistically represent what you're doing anyways: Your receipt isn't a set of arbitrary prices, it's a mapping of currencies to totals in that currency.

If you really want to go all OO on it you could make a Receipt class, e.g.:

public class Receipt {
    private Map<Currency,Price> totals;
    ...
}

Then add various getters and setters that hide the fact that you're using a Map internally (e.g. List<Currency> getCurrencies (), and Price getTotal (Currency c), and things like that). But you don't really gain much from doing that in your simple example aside from the learning experience.

As for your equals and hashCode in Price:

Will it be a bad choice to override equality methods in this way and what possible pitfalls it may cause in the future?

Of course it would be a bad choice. Two objects shouldn't be equal unless they are equal, and comparing only the currency unit of a price doesn't make sense, because $3 USD and $400 USD are not equal. Implement equals and hashCode according to your actual business rules:

• For equals two prices aren't equal unless their amount and currency unit are equal. So compare both.
• For hashCode it's a little more lenient, the only real constraint is if equals() returns true for two objects, then their hashCode() should be the same. But it's also OK if two non-equal objects produce the same hash code. Usually you pick a hashCode() that gives a good distribution of hash codes, so for your case I'd either return e.g. cash.hashCode(), or some combination of the cash and currency hash codes.

Because of that, it's not a great idea to use a Set -- doing so means you'd have to have an equals() implementation that doesn't really line up with reality.

Hope that helps.