3

According to Is "avoid the yo-yo problem" a valid reason to allow the "primitive obsession"?, I should define "price" like this:

public class Main{
    private Price price;
}

public class Price{
    private double priceValue;
    public static void roundToValidValue(double priceValue){
    }
}

instead of 

public class Main{
    private double price;
}

public class PriceHelper{
    public static double roundToValidValue(double priceValue){
    }
}

because I should encapsulate the logic about Price into a class to increase maintainability. However, I found it seems go against the goal of "use most abstract type as possible" (Why define a Java object using interface (e.g. Map) rather than implementation (HashMap)) :

As I understand, one of the goal of "use most abstract type as possible" is to reduce the coupling between classes, which lets the clients avoid using methods that they don't call, and hence the clients wouldn't depend on the methods that they don't use. However, back to the Price class, I found that some clients don't call "roundToValidValue" at all. If I follow the "avoid primitive obsession" rule and encapsulate "roundToValidValue" into a new class, the clients that don't call "roundToValidValue" at all would also depend on "roundToValidValue", which seems go against the goal of "decoupling" that suggested by "use most abstract type as possible".

Also, according to Should we define types for everything?, I should avoid using primitive types to represent business model directly, ie: I should change

public class Car{
    private double price;
    private double weight;
    public void methodForPrice(double price){
    }
}

into

public class Class{
    private Price price;
    private Weight weight;
    public void methodForPrice(Price price){
    }
}

which can avoid the situation that calls the wrong method ,ie :methodForPrice(weight). However, according to Why define a Java object using interface (e.g. Map) rather than implementation (HashMap), it suggests using most abstract type as possible, ie:

public LinkedHashMap<String,Stock> availableStocks;
public HashMap<String,Stock> unusedStocks;

public class Main{
    public void removeUnusedStocks(Map<String,Stock> unusedStocks){
    }
}

which I found it may suffer from the problem of "primitive obsession" : I may call "removeUnusedStocks(availableStocks)" wrongly. So I think use more specific type:

public class Main{
    public void removeUnusedStocks(LinkedHashMap<String,Stock> unusedStocks){
    }
}

is better even if both versions works idendically and hence I think "use most abstract type as possible" go against the goal of "avoid primitive obsession".

As the result, I think the goal of "avoid primitive obsession" and "use most abstract type as possible" go against each other and hence "avoid primitive obsession" and "use most abstract type as possible" contradicts each other, is that true?

aacceeggiikk
  • 707
  • 1
  • 5
  • 6
  • I think there is a bug in `roundToValidValue()` in the first example. It is static *and* void, which means it will not have any effect. Was it supposed to return a `Price` object? – JacquesB Feb 06 '20 at 07:45
  • I don't get what you mean with "I may call `removeUnusedStocks(availableStocks)` wrongly ... even if both versions works identically" - how is it wrong if it works correctly? Why is the more specific type better if both works? – JacquesB Feb 06 '20 at 07:49

4 Answers4

8

I think the important piece to understand in your examples is that a Price should not be the same as a double--for starters, doubles can't represent monetary amounts correctly. But Price has other restrictions--it can't be negative, it can't have more than 3 decimal places (in the US), etc. Those are type invariants. They prevent you from making mistakes by enforcing conditions at the type level. The same is true of Weight. Using them together prevents you from adding Price to Weight--which is a good thing.

For those reasons, a double is not an abstraction over Price or Weight. They're completely different types with different purposes. You can't use a Price the same way you can use a Weight, which means you can't use a double (even though that may be the type you use internally to store the value). Abstractions should hold the same important invariants as the concrete types. HashMaps are Maps and can be used in any way that a Map can be used.

mgw854
  • 1,818
  • 10
  • 11
  • There are even patterns for that: https://www.martinfowler.com/eaaCatalog/money.html ;-) – Christophe Feb 06 '20 at 08:44
  • “It can’t have more than 3 decimal places (in the us)” [oh really?](https://softwareengineering.stackexchange.com/a/344327/131624) – candied_orange Feb 06 '20 at 09:52
  • 1
    Clearly we should be using the most abstract `Price` interface, and put that constraint in the more specific `ItemCashPrice` (as opposed to `BulkPrice`, `CommodityPrice`, etc.) where it belongs – Useless Feb 06 '20 at 09:58
3

It's worth pointing out that double is not an abstract type. It's a concrete one. Price isn't an abstract type either.

If you had some AbstractScalarType which abstracts across all scalar types, then AbstractScalarType could still specify (but not necessarily implement) a method roundToValidValue. That said, there is probably little value in such a class in your application. Don't invent pointless abstract classes just so you can inherit from them.

The principle of using the most abstract type primarily refers to which types your methods should accept. If you have a method that analyzes a list of things to produce a summary of what's in the list, then there's no reason to specify that your method only works on one specific kind of list (e.g. a singly-linked sorted list). It should accept any kind of list.

Simon B
  • 9,167
  • 4
  • 26
  • 33
  • +1 for "Price is not an abstract type". It's a (hopefully) well designed CONCRETE type to handle prices, hopefully better suited to handle prices than using "double". – gnasher729 Feb 08 '20 at 15:15
1

The author of the rule "use most abstract type" probably did not mean replacing single-field classes with their content. After all, double is not a supertype to the Price. I think they would rather meant some interface Price which could be implemented with different PriceImpl classes. If it is still justified to be used here is another question (by my opinion it is not).

max630
  • 2,543
  • 1
  • 11
  • 15
0

"define the Java object using the interface rather than the implementation", is short version of

Use most abstract type at declaration and most specialized type at creation

So when you say use most abstract type as possible, you forget at declaration. The main idea is to

  1. Easily change implementation if needed
  2. Use the same method for all sub-cases.

In that case, the removeStocks(Map<String,Stock> stocks) could be used both for removing unused stocks (e.g. from available at store) and for removing available stocks (e.g. from cart). Please note that type safety could prevent from many errors but not from all: store.removeStocks(available) is logical bug. Declaring availableStocks as LinkedHashMap only for that reason would be a bigger error (both performance and maintainability).

From other hand, it's important part as possible. Since you have restrictions for weight, it's impossible to use primitive double. Since nothing changes if you change LinkedHashMap<> availableStocks to HashMap<> availableStocks - it's better to use more general Map

TL;DR If they contradict then most probably you use one of them in wrong way. It's time for review.

ADS
  • 164
  • 4