Method requires concrete implementation of collection. Should I change all upstream methods to return concrete implementations?

softwareengineering.stackexchange https://softwareengineering.stackexchange.com/questions/341433

  •  07-01-2021
  •  | 
  •  

Question

I have a method processDataAssumingLinkedHashMapInput() that processes a Map. The Map must be a LinkedHashMap ordered by values. Data comes from getStrIntMap(query). This method gets resultSet from SQL and puts it in a LinkedHashMap. Here is the code:

public void processDataAssumingLinkedHashMapInput(){
    String query = "select MYID, MYVALUE from MYTABLE order by MYVALUE";
    Map<String, Integer> map = SQLTools.getStrIntMap(query);

    for (Map.Entry<String, Integer> entry : rawEntryToSortOrderMap.entrySet()) {
        //do something, assuming that values are ordered
    }
    //do more stuff
}

//SQLTools method, used by multiple other classes
public static Map<String, Integer> getStrIntMap(String query){
    Map<String, Integer> map = new LinkedHashMap<>();
    //ResultSet to map 
    return map;
}

My concern is that if someone decides to change getStrIntMap(String query) to Map<String, Integer> map = new HashMap<>(); (for instance for performance reasons) it will break processDataAssumingLinkedHashMapInput(). I can change return type of getStrIntMap to concrete implementation, but than it wouldn't look good and someone will change it back to an abstract Map. I can create two essentially identical methods one of which returns Map and the other returns LinkedHashMap, but this would break DRY principle. I can reorder data in the beginning of processDataAssumingLinkedHashMapInput method, but this again violates DRY rule. What is the best practice for this case?

Was it helpful?

Solution

Use a little IoC. Refactor to allow the caller to inject the map instance, which will allow other developers to use the more efficient map and allow you to use the map with the features you want.

public static void fillStrIntMap(String query, Map<String, Integer> map){
    //ResultSet to map 
}

OTHER TIPS

I suggest adding a method to the library that populates a caller-supplied empty Map from the supplied query.

Keeping things DRY, the getStrIntMap method would invoke this other method passing in a new LinkedHashMap, and returning that Map (as an interface, so its signature remains unchanged).

Then the caller can choose the standard getStrIntMap which returns the Map interface for loose coupling, or, the caller can choose the other passing in a Map of its choice so it can have clear guarantees that the map is indeed a LinkedHashMap, and yet, the library API never has to expose concrete types.

One possibility is to create a new abstract type called OrderedMap that extends abstract Map and return that. That way the method still returns an abstraction but now it is enforced that it returns an ordered map type. Existing callers can still treat the returned object as a basic map if they want, or be confident that the returned object is ordered with the implementation protected against being idly changed to return eg HashMap.

Licensed under: CC-BY-SA with attribution
scroll top