1

I recently wrote a program in which I wrapped a map data structure to increase the readability of code like so :

  package wrappers;

import model.primary.customer.Customer;

import java.util.HashMap;

public class CustomerMap extends HashMap<Integer, Customer> {

}

I needed all the functions of the hashmap , and wanted to keep it wrapped inside a class so that it's readable... that's why I used a wrapper class.. However, a programmer told me that this is ironically making my code less readable / understandable ...

Here is another data structure for which I used a wrapper :

package wrappers;

import model.primary.customer.AgeRange;

import java.util.EnumMap;
import java.util.HashMap;


public class MovieAgeRangeMap extends HashMap<Integer, EnumMap<AgeRange, Integer>> {
}

I still believe that wrapping data structures which are complicated and long , like the last one mentioned, makes them readable ...

What is the threshold of complication , after which one SHOULD put datastructues in wrappers ? What's a good rule of thumb ?

Also, when I made the project , a fellow programmer told me that I had a LOT of packages , which was not good , as I had overcomplicated the solution .... However, I did that because making a lot of packages and putting logically related code in a package made the project organized and intuitive . So , what's the problem with having a lot of packages ? What's the ideal package number for my project ?

I'm more than happy to get any sort of constructive feedback about any shortcomings in my project!!

my project

3

Part of the problem here is that Java does not have type aliases. Subclassing is sometimes used to introduce a shorted name for a complex type, but it is not the same. Quite literally – you have introduced a new type. While using this subclass is fine for local variables, it is unsuitable for public interfaces (e.g. method signatures).

In addition to that problem, consider whether an abstraction is worth it. The goal of an abstraction is to hide some details. Using the abstraction is supposed to be easier than having to understand all details. But first we have to understand the abstraction, so this also adds some cost. If an abstraction causes more effort than it saves, it's not worth it.

For your wrapped collections, every Java programmer knows what a HashMap is. No one knows what a CustomerMap is, so we first have to learn about that. Ok, here it's just a HashMap with Integer keys and Customer values. The irony is, this is not even an abstraction – no details are really hidden. Yes, the name is shortened, but on a semantic level all the details are still there. I would therefore agree that this abstraction is not worth it, and that your code might be easier to read if you spell the full type out.

There is an alternative: develop a real abstraction that provides functionality in the problem domain and hides unimportant details. One detail might be: this uses a HashMap internally.

E.g. consider this design:

public final class Customers implements Iterable<Customer> {
  private final HashMap<Integer, Customer> storedCustomers = new HashMap<>();

  public void addCustomer(Customer customer) {
    storedCustomers.put(customer.id, customer);
  }

  public Customer getCustomerById(int id) {
    return storedCustomers.get(id);
  }

  public Iterator<Customer> iterator() {
    return storedCustomers.values().iterator();
  }
}

Users of this class need to understand far less than the users of a HashMap so this abstraction ~might~ be worth it. This will be more clearly the case for your MovieAgeRangeMap because the abstraction could handle creation of the nested EnumMap, thus shielding the user from this complexity.

Then again, if an abstraction is only used on one place it's likely not worth it because it doesn't save that much effort when understanding the code. In each case you have to balance various forces on your design when trying to simplify the code. For example:

  • Cohesion: smaller components are easier to understand VERSUS fewer components are easier to understand

    or phrased in reverse: too much code in one place is difficult to understand VERSUS too many external references are difficult to understand

  • Development effort: abstractions save effort through reuse VERSUS building reusable abstractions takes effort

  • Mental overhead: abstractions save complexity by abstracting over details VERSUS abstractions introduce complexity by introducing another concept that has to be learned first

Either extreme is bad, but balancing these factors is difficult.

  • Your wrapper classes can be criticized because they save very little complexity but introduce another concept.

  • Your high number of packages/classes can be criticized for the same reason: many concepts that each add very little value. For model classes this is understandable because they represent a meaningful concept. But this is problematic if functionality is distributed across multiple packages. Identifying the seams along which functionality should be split up gets easier with experience. If a class is needed only as an implementation detail of another class, maybe a private class or a nested class would be appropriate.

2

Short answer: In the case given, I would prefer the bare Java HashMap. Let me elaborate why.

Programming for readability is a very important goal. The tricky question is how to interpret readability.

In your case, the wrapper's intent isn't to add functional value to the underlying map, it's just giving a prettier name to it. Which of the two names will be easier to grasp for the reader? Of course, that depends on the reader, but let's assume the reader to be the next developer to modify that function or fix a bug, so most probably the reader will be an experienced Java programmer. So I'll comment based on my 20 years of Java development.

  • Seeing a HashMap<Integer, Customer>, I know that it has to do with Customers, and that it maps Integers to Customers (and I'll guess that the Integers might be the Customer identifiers, as I have seen that pattern thousands of times). Having used HashMaps quite often, I immediately know what I can do with it.

  • Seeing a CustomerMap, I can only guess that it has to do with Customers and might be something like a map (in the collections or the geographical sense?). Here it takes me time (maybe a minute to look inside the CustomerMap class) to understand that it's just the familiar HashMap, just with a pretty name.

Now, is it always wrong to create wrappers? NO!!

If the new name represents a domain concept that existed before you wrote the class (maybe it was mentioned in some requirements paper), then it's a completely different situation. You start with the domain concept, e.g. a CustomerMap as part of some printout report, and create a class to represent that. By analysis you maybe find that a HashMap provides all the functionality you need, and even with method names that match the domain-specific terminology well enough so that you can keep them. In that situation, I'd prefer to see the HashMap renamed (wrapped) to CustomerMap, as the HashMap just happens to be the implementation of a domain concept.

Or, if the wrapper class adds or removes functionality or transforms parameters or results, a named wrapper is absolutely perfect.

So, sorry to disappoint you in this case, the time you spent for the wrapper meant to improve readability didn't really achieve that (at least for me).

The second question is about packages. Packages can be used to provide some access restrictions (with the auto visibility), although that isn't widely used. Besides that, it's a means of bringing structure into a big bunch of classes. There's a lot of personal style involved, so I can't give you clear advice here.

Anyway, your approach to care about readability (and sometimes leave room for improvement) is much better than the zillions of programmers who write "clever" or "optimized" code and end up with crazy, slow-running spaghetti.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.