Two Key HashMap

Question

Background

Would like to create a two-key map that is fairly generic. In the existing system, there are a number of ways to name a parameter for a report. Rather than re-writing each report (too much effort), we'd like to simply map the old parameter names to new names.

For example, old names might include STARTDATE, FROM_DT, P_DATE_START, and countless more variations thereof each being mapped to a single, equivalent P_START_DATE.

By using a consistent naming scheme, we could, for example, detect the data type (e.g., by following a convention such as ending parameters in the data type, as in _DATE) and then automatically generate a corresponding web page complete with validation for the report's form submission. This is otherwise an arduous task with an inconsistent naming scheme across hundreds of legacy reports.

Code

The existing code resembles:

public class TwoKeyMap {
 private enum Parameter {
 P_START_DATE,
 P_END_DATE;
 }
 private static final Map<String, Map<Enum, String>>
 REPORT_PARAMETER_MAP = new HashMap<String, Map<Enum, String>>() {
 {
 put("ReportName", new HashMap<Enum, String>() {
 {
 put(Parameter.P_START_DATE, "P_DATE_START");
 put(Parameter.P_END_DATE, "P_DATE_END");
 }
 });
 }
 };
 /**
 * Retrieves the old parameter name based on the report using the new parameter name.
 *
 * @param report - The report name having a parameter mapped to a new name.
 * @param key - The new name of the parameter.
 * @return The report's old parameter name.
 */
 public static String lookupParameter(String report, Parameter key) {
 return getReportParameterMap().get(report).get(key);
 }

Question

How would you improve the code so that it is easier for new and intermediate Java developers to maintain?

For example, would this be easier:

TwoKeyMap tkm = new TwoKeyMap();
tkm.put( ReportNames.REPORT1, Parameter.P_START_DATE, "P_DATE_START" );
tkm.put( ReportNames.REPORT1, Parameter.P_END_DATE, "P_DATE_END" );
tkm.put( ReportNames.REPORT2, Parameter.P_START_DATE, "P_FROM_DT" );
tkm.put( ReportNames.REPORT3, Parameter.P_START_DATE, "DATE_FROM" );
tkm.put( ReportNames.REPORT4, Parameter.P_START_DATE, "P_BEGIN_DATE" );

Related Links

• http://commons.apache.org/collections/apidocs/org/apache/commons/collections/keyvalue/MultiKey.html

• tb-: I am not sure if codereview is the right place. You are more asking for a solution, not to review some code. Nevertheless I do not really understand the problem. If you want to have 2 keys for a Hashmap, you should create some container class (with 2 fields in this case) which implements equals and hashcode properly. In some cases, you could avoid this class if you use field1<separator>field2 as key, with separator some unique constant not appearing in field1 or field2.
• Eric P.: Guava has a data structure called a Table that you can reference: docs.guava-libraries.googlecode.com/git/javadoc/com/google/…

Answer 1

By using a consistent naming scheme, we could, for example, detect the data type (e.g., by following a convention such as ending parameters in the data type, as in _DATE) and then automatically generate a corresponding web page complete with validation for the report's form submission.

It is not the best idea to rely on a naming scheme to do this. It looks like you need some metadata for your parameters.

You already gave an example using an enum, you could go further an add the metadata directly in the enum:

public enum Parameter {
 P_START_DATE(Date.class),
 P_END_DATE(Date.class);
 private final Type type;
 Parameter(Type type) {
 this.type = type;
 }
 public Type getType() {
 return type;
 }
}

Then there is a perfect Map implementation when using enum keys: EnumMap. You could even subclass it to have a type representing all the parameters for one report. I guess you use one report at a time so it is easier to first get the report parameters and then get the individual parameters:

public class ReportParameters extends EnumMap<Parameter, String> {
 final Report report;
 public ReportParameters(Report report) {
 super(Parameter.class);
 this.report = report;
 }
}

Finally to go further with type safety you could add an enum representing the reports so you could store all the reportsParameters instances in another enumMap:

public enum Report {
 FIRST_REPORT,
 SECOND_REPORT;
}

and:

Map<Report, ReportParameters> reportParametersMap = 
 new EnumMap<Report, ReportParameters>(Report.class);

Using this would look like (with static imports of the enums):

ReportParameters firstReport = new ReportParameters(FIRST_REPORT);
reportParametersMap.put(firstReport.report, firstReport);
firstReport.put(P_START_DATE, "P_DATE_START");
firstReport.put(P_END_DATE, "P_DATE_END");
ReportParameters secondReport = new ReportParameters(SECOND_REPORT);
reportParametersMap.put(secondReport.report, secondReport);
secondReport.put(P_START_DATE, "FROM");
secondReport.put(P_END_DATE, "TO");

With that you can also add more helper methods if you want or need (like a check if you have declared all mappings, or helper methods to populate or access the information.)

• Dave Jarvis: I prefer this solution because it does not depend on external libraries.
• palacsint: +1 and thanks for the answer! An improvement idea: The ReportParameters class, instead of extending EnumMap, could have delegate methods for get and put with an EnumMap<Parameter, String> field. This way the get method could be public String get(final Parameter key) { ... } (instead of public String get(final Object key) { ... }) which would improve type safety and prevent completely invalid calls, like: reportParametersMap.get("string1").get("string2");.
• pgras: @palacsint: good suggestion, in fact I thought Map<K,V> had a get(<K> key) method but it is get(Object key)... BTW there is a lot to improve in my solution but to improve in the right direction one should know more about how the code will be used.

Answer 2

I'd go with something which is similar to the mentioned MultiKey: create a ReportParameter class and use it as the key of the map:

public class ReportParameter {
 private final String reportName;
 private final Parameter parameter;
 public ReportParameter(final String reportName, final Parameter parameter) {
 this.reportName = reportName;
 this.parameter = parameter;
 }
 // equals and hashCode here
}

(Make sure that is has proper hashCode and equals methods.)

I think it's easier to understand than the nested maps (for example, Map<String, Map<Enum, String>>).

Furthermore, I'd consider creating and using different types (!= string) for the report name and the parameter names. String identifiers are hard to follow and it's easy to accidentally mix them up. Different types (for example, enums, like Parameter) and type safety help here.

A sidenote from Code Complete 2nd Edition, Chapter 5: Design in Construction, Value of Information Hiding, page 96:

[...]

Most programmers would decide, "No, it isn’t worth creating a whole class just for an ID. I’ll just use ints."

[...]

Rather, the difference is one of heuristics—thinking about information hiding inspires and promotes design decisions that thinking about objects does not.

• palacsint: @DaveJarvis: (1) I don't exactly understand this. Anyway, is memory consumption really an issue? (2) I've updated the answer a little bit.
• mheinzerling: @DaveJarvis There is always exactly one instance of every String content in Java. All Strings with the same content refer to the same internal string. If you can reduce this to an Enum it should be even cheaper.
• pgras: @mnhg: you can have several String instances with the same content, try this:String one = "one"; String alsoOne = new String("one"); System.out.println("same: " + (one == alsoOne) + ", equals: " + (one.equals(alsoOne)))
• mheinzerling: @pgras You are comparing the wrong objects: String.intern()
• pgras: @mnhg: you wrote "There is always exactly one instance of every String content in Java." maybe I don't exactly understand what you mean by "String content". I know you can intern() Strings but as long as you don't the "content" of the string is a private char array from the String. So in my example both strings have their own copy of the "one" content...

Answer 3

A possible solution using Table as the backing structure (you could use your existing TwoKeyMap). This adds a touch of abstraction over what you already have to make the intent a little more clear for someone new. I've also included a couple different setup possibilities.

import com.google.common.collect.Table;
public class ParamTranslator {
 private Table<ReportName, Parameter, String> table = HashBasedTable.create();
 public ParamTranslator addRule(ReportName report, Parameter param, String oldName) { 
 table.put(report, param, oldName);
 return this; // Method chaining
 }
 public String getOldFor(ReportName report, Parameter param) {
 return table.get(report, param);
 }
}
// ... somewhere in config land
for(String line : /*list of file lines*/) {
 String[] lineParts = line.split(","); // store as a csv?
 // (more validation would need to be added here)
 translate.addRule(ReportName.valueOf(linesParts[0]), 
 Parameter.valueOf(lineParts[1]), lineParts[2]);
}
// ... maybe inline using method chaining
ParamTranslator translate = new ParamTranslator() 
 .addRule( ReportName.REPORT1, Parameter.P_END_DATE, "P_DATE_END")
 .addRule( ReportName.REPORT2, Parameter.P_START_DATE, "P_FROM_DT")
 .addRule( ReportName.REPORT3, Parameter.P_START_DATE, "DATE_FROM")
 .addRule( ReportName.REPORT4, Parameter.P_START_DATE, "P_BEGIN_DATE");
// ... later in the program
String oldParam = translate.getOldFor(reportName, dateParam);

• Paul Bellora: Note that Table is an interface, so new Table<>() would not compile. You should have a look at the different implementations and recommend one.
• Eric P.: A silly mistake on my part, updated with the proper factory call.
• palacsint: I think it is easiest to understand version. Furthermore, the getOldFor() method accepts only the proper types. Thanks for the answer and enjoy the bounty and +1 :)