Inheriting and encapsulating collection classes in Java

Question

Suppose I have the following types of data:

class Customer {
  String id; // unique
  OtherCustData someOtherData;
}

class Service {
  String url; // unique
  OtherServiceData someOtherData;
}

class LastConnection {
  Date date;
  OtherConnData someOtherData; // like request or response
}

Now I need to remember when each of the customers connected to each of the services.
I would make the structure:

Map<Customer, Map<Service, LastConnection>> lastConnections;

Or, to be able to search by ids and not have to write all the equal() and hashCode():

Map<String, Map<String, LastConnection>> lastConnections;

Now I could access the LastConnection data by

LastConnection connection = lastConnections.get(custId).get(srvUrl);

All this seems ugly, especially that I have to pass it as parameters to tens of methods expecting map of maps of LastConnections, so I'm thinking of creating my own classes which would look something like that:

class CustomerConnections extends HashMap<String, LastConnection> {
}

class AllConnections extends HashMap<String, CustomerConnections> {
    public LastConnection get(String custId, String srvUrl) {
        return get(custId).get(srvUrl);
    }
}

Ok, I learned already that inheritance is 3v1l, so let's try composition:

class CustomerConnections {
    Map<String, LastConnection> customerConnections;
    LastConnection get(String srvUrl) { 
        return customerConnections.get(srvUrl);
    }
    ... // all other needed operations;
}

class AllConnections {
    Map<String, CustomerConnections> allConnections;
    public LastConnection get(String custId, String srvUrl) {
        return get(custId).get(srvUrl);
    }
    public CustomerConnection get(String custId) {
        return allConnections.get(custId);
    }
    ... // all other needed operations;
}

The problem is that I'm not sure what would be the best approach respecting SOLID principles and all the best practices. Creating classes that do nothing except extending already existing collections seems like multiplying entities beyond necessity, but would make my code more clear (Especially when there are next levels - like Map of AllConnections by month and so on). Any directions?

Answer 1

You could create a class that implements Map<K,V>, and internally delegate to a container map:

class CustomerConnections implements Map<String,LastConnection> {
    private Map<String, LastConnection> customerConnections;

    @Override
    public LastConnection get(Object srvUrl) { 
        return customerConnections.get(srvUrl);
    }
    // all other needed operations;
}

The nice thing with this approach is that you can pass around a Map which has a standard, well-defined contract, but you avoid extending library classes, which is often frowned upon.

EDIT : As pointed out below, this does have the downside of requiring you to implement a lot of methods that do nothing short of delegate to the underlying collection

Answer 2

Creating classes that do nothing except extending already existing collections seems like multiplying entities beyond necessity

I would change extend to encapsulate. You are hiding the details of how this information is stored. Clients of your class don't need to know how you are providing them with the Customer connection history. I think this is a good idea since you can change the underlying model without having clients of the api change their code.

but would make my code more clear

This is great and is a good reason to do this. YourClass.getCustomerConnection(cId) is much clearer than yourCollection.get(id).get(id).getConnection(). You need to make the life of the people using this code easier, even if you are that person.

(Especially when there are next levels - like Map of AllConnections by month and so on)

Good, then you are planning ahead and making your code extensible. Which is good OO practice. In my opinion the conculsion you have reached on your own is what I would do.

Answer 3

Why not use custId+"#"+srvurl as a key in a simple Map<String, LastConnection>?

Or use a Tuple or Pair class as key that contains the two IDs and implements hashCode() and equals() - the "cleanest" OO solution.

Answer 4

I would create a dedicated object for storing this information. What you're creating is a manager object, rather than a simple collection.

I wouldn't make it derive from a Map or other well-known collection class, since the semantics of how you store this info may change in the future.

Instead implement a class which ties together a customer and its connection, and inside that class use the appropriate collection class (that you're at liberty to change later without affecting the interface and the rest of your code)

Your customer/connection manager class is more than a simple container. It can store meta-data (e.g. when was this relationship established). It can perform searches on connections given customer info (if you require). It can handle duplicates how you require, rather than how the underlying collection class handles them etc. etc. You can easily slot in debugging/logging/performance monitoring to easily understand what's going on.

Answer 5

Why are you maintaining the relationships between objects outside of those objects.

I'd suggest something like the following:

public class Customer 
{
  public List<LastConnection> getConnectionHistory() 
  {
    ... 
  }

  public List<LastConnection> getConnectionHistory(Service service) 
  {
    ...
  }

  public List<LastConnection> getConnectionHistory(Date since) 
  {
    ...
  }
}

public class LastConnection
{
  public Date getConnectionTime() 
  {
    ...
  }

  public Service getService()
  {
    ...
  }
}

You then pass List<Customer> to the methods that use this information.

Answer 6

I think that you should also consider how the collections are going to be used and how the data is obtained:

• If they are simple result sets to be displayed on a page (for example), then using standard collections seems reasonable - and you can then use lots of standard libraries to manipulate them.
• On the other hand, if they are mutable and any changes need to be persisted (for example), then encapsulating them within your own classes might be preferable (which can then write changes to databases, etc).

How do you get and persist your data? If it is stored in a database, then maybe you can use SQL to select LastConnections by customer and/or service and/or month, rather than having to maintain the datastructures yourself (and just return simple lists or maps of connections or counts of connections). Or maybe you don't want to run a query for each request, so do need to keep the whole datastructure in memory.

Encapsulation is generally a good thing though, particularly as it can help you to adhere to the Law of Demeter - maybe you can push some of the operations that you will perform on the collections back into the AllConnections class (which is effectively a DAO). This can often help unit testing.

Also, why is extending HashMap considered evil here, considering that you only want to add a trivial helper method? In your code for AllConnections, AllConnections will always behave in the same way as a HashMap - it is polymorphically substitutable. Of course, you are potentially locking yourself into using HashMap (rather than TreeMap, etc), but that probably doesn't matter since it has the same public methods as Map. However, whether you would actually want to do this really does depend upon how you intend to use the collections - I just don't think that you should automatically discount implementation inheritance as always bad (it just usually is!)

class AllConnections extends HashMap<String, CustomerConnections> {
    public LastConnection get(String custId, String srvUrl) {
        return get(custId).get(srvUrl);
    }
}