4
\$\begingroup\$

In a Java 8 project, I'm using several maps, many of which contain static or default content. In Java 9 and newer, there's the convenient Map.of() method and I want something similar convenient. So, I came up with the following implementation:

public class CollectionUtils {

    private static final String INCOMPATIBLE_TYPE_MSG_TMP = "The %sth argument is not compatible with the %s type %s";

    /**
     * Shortcut to create a map
     * @param keyClass      type class of keys
     * @param valueClass    type class of values
     * @param keysAndValues array with keys and values (even indices: keys, odd indices: values)
     * @param <KeyT>        type of keys
     * @param <ValueT>      type of values
     * @return unmodifiable map
     */
    public static <KeyT, ValueT> Map<KeyT, ValueT> map(
            Class<KeyT> keyClass,
            Class<ValueT> valueClass,
            Object... keysAndValues) {
        if (keysAndValues.length % 2 != 0) {
            throw new IllegalArgumentException("The number of keys and values must be even");
        }
        Map<KeyT, ValueT> outputMap = new HashMap<>();
        KeyT key = null;
        for (int i = 0; i < keysAndValues.length; i++) {
            Object keyOrValue = keysAndValues[i];
            if (i % 2 == 0) {
                if (!keyClass.isInstance(keyOrValue)) {
                    String message = String.format(INCOMPATIBLE_TYPE_MSG_TMP, i, "key", keyClass.getName());
                    throw new IllegalArgumentException(message);
                }
                key = keyClass.cast(keyOrValue);
            } else {
                ValueT value;
                try {
                    value = valueClass.cast(keyOrValue);
                } catch (ClassCastException e) {
                    String message = String.format(INCOMPATIBLE_TYPE_MSG_TMP, i, "value", valueClass.getName());
                    throw new IllegalArgumentException(message, e);
                }
                outputMap.put(key, value);
            }
        }
        return Collections.unmodifiableMap(outputMap);
    }
}

I did quite a lot of tinkering to get this implementation and I believe there's still room for improvement. For example, the first two arguments must be the key and value classes to get the types correctly, which Java 9's Map.of() doesn't require. Besides implementational improvements, I'm also open to comments regarding efficiency, security and anything sub-optimal with this implementation.

\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

I don't see why you sould want to throw an IllegalArgumentException when the error case is actually a literal ClassCastException. This is actually counterproductive as you are hiding the fact that the error was caused by class incompatibility and replacing it with a generic illegal argument exception. For example, the java.util.List.remove(Object) throws a ClassCastException if you feed it the wrong type of objects.

Since you know that the length of the array is divisible by two, you can pick both key and value from it at the same time without having to track the state the loop is in, and advance the loop counter by 2.

The loop becomes quite simple now:

Map<KeyT, ValueT> outputMap = new HashMap<>(keysAndValues.length / 2);
for (int i = 0; i < keysAndValues.length; i += 2) {
    outputMap.put(
        keyClass.cast(keysAndValues[i]),
        valueClass.cast(keysAndValues[i + 1]));
}

Is your code actually a performance bottle neck? There is not much one can do to make converting an array to map much faster. Giving the HashMap it's initial size to perevent it from doing unnecessary rehashings is one thing.

\$\endgroup\$
6
  • \$\begingroup\$ Wow, the code becomes so much simpler now, thank you! I mainly use this code in static areas, such as to initialize static class members, not really performance critical. But I was hoping someone would catch any language- or runtime-specific problems in my approach, which puts forward my understanding of the language. \$\endgroup\$ Jan 11 at 6:26
  • 1
    \$\begingroup\$ @Green绿色 If absolute performance is not a critical requirement, you should look into the builder pattern: minborgsjavapot.blogspot.com/2014/12/… . This seems like a case where a builder is actually useful. They're a bit more cumbersome to use than pure Object arrays, but they provide compile time type checking. \$\endgroup\$ Jan 11 at 7:12
  • \$\begingroup\$ Thanks for your suggestion. I haven't thought about applying the Builder pattern to this problem, but from a usage perspective, the Builder approach looks much better than mine. \$\endgroup\$ Jan 11 at 11:13
  • 1
    \$\begingroup\$ The use of new HashMap<>(keysAndValues.length / 2) will still cause a resize, because it fails to take into account the default loadfactor of 0.75. \$\endgroup\$ Jan 11 at 17:56
  • \$\begingroup\$ @MarkRotteveel Good point. It's a bit silly desing from the API developers as it works differently than, say, ArrayList initial capacity. \$\endgroup\$ Jan 12 at 6:27
3
\$\begingroup\$

Not a fan of hacking compile-time type-safety, so I'm just going to mention an alternative.

Pair class is a classic, you can find it in most libraries like apache-commons or even in JDK in a way (Map.Entry), wrote it here just to demonstrate.

public class Pair<K, V> {
    public K key;
    public V value;

    public Pair(K key, V value) {
        this.key = key;
        this.value = value;
    }

    public static <K, V> Pair<K, V> of(K key, V value) {
        return new Pair<>(key, value);
    }
}

public class CollectionUtils {
    public static <KeyT, ValueT> Map<KeyT, ValueT> map(Pair<KeyT, ValueT>... input) {
        Map<KeyT, ValueT> outputMap = new HashMap<>(input.length);
        for (Pair<KeyT, ValueT> pair : input)
            outputMap.put(pair.key, pair.value);
        return Collections.unmodifiableMap(outputMap);
    }
}

public static void main(String[] args) {
    Map<String, Integer> map = CollectionUtils.map(
            Pair.of("a", 1),
            Pair.of("c", 2)
    );
    
    System.out.println(map);
}
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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