Map<K, Object> => Map<K, String>

Given a Map<K, Object>, I'm returning Map<K, String> or throwing an Exception for at least one non-String value.

     private static <K> Map<K, String> validate(final Map<K, Object> map)
throws NonStringValueException {
final List<Object> invalidValues = new ArrayList<Object>();
final Map<K, String> result = new HashMap<K, String>();

for(Map.Entry<K, Object> entry : map.entrySet()) {
if(!(entry.getValue() instanceof String)) {
}
else {
result.put(entry.getKey(), (String) entry.getValue());
}
}

if(!invalidValues.isEmpty()) {
throw new NonStringValueException("Invalid Values:
must be String type: " + invalidValues);
}

return result;
}


Besides using instanceOf and then casting, is there a better way to write this method?

Some simple things first - there's no need to repeat the generic type of variables on the right-hand-side of a new assignment. Java 7 introduced the "diamond operator" <> and the generic type is inferred. So, lines like:

final List<Object> invalidValues = new ArrayList<Object>();
final Map<K, String> result = new HashMap<K, String>();


can be just:

final List<Object> invalidValues = new ArrayList<>();
final Map<K, String> result = new HashMap<>();


Your use of the static method and generic type <K> is good. I like how you have made the variables final in the method too. Additionally, your use of the for-each loop on the Map's entries is also good.

A comment on your generics, your input map should be Map<K, ?> instead of Map<K, Object> .... the ? allows more flexibility.

Java introduced a couple of helper methods on Class a while back, and I think you could use these to make this method even more generic.... why force it to only be a String?

Consider the following two methods:

private static <K, V> Map<K, V> validate(Class<V> coerce, final Map<K, ?> map)
throws IllegalArgumentException {
final List<Object> invalidValues = new ArrayList<>();
final Map<K, V> result = new HashMap<>();

for(Map.Entry<K, ?> entry : map.entrySet()) {
if(coerce.isInstance(entry.getValue())) {
result.put(entry.getKey(), coerce.cast(entry.getValue()));
} else {
}
}

if(!invalidValues.isEmpty()) {
throw new IllegalArgumentException("Invalid Values: must be " + coerce.getName() +  " type: " + invalidValues);
}

return result;
}

public static <K> Map<K, String> validate(Map<K, ?> map) {
return validate(String.class, map);
}

• Thanks for your thoughtful answer - I liked the generic Class<V> that you taught me. Lastly, what are your thoughts about using a checked exception instead - to let the compiler help us out more? – Kevin Meredith Jul 14 '15 at 14:15
• The checked exception is very much dependent on your context. I am not sure whether it is "right", but I seldom create custom exceptions any more, and rely on native exceptions. Further, I am using unchecked more often too (like IllegalArgumentException) because it helps when using Java 8 streams. – rolfl Jul 14 '15 at 14:18
• allows more flexibility. And that's because of what Joshua Bloch says in Item 23 of Effective Java: you can't put any element (other than null) into a Collection<?>.? That's the reason for using ? over Object in your answer? – Kevin Meredith Jul 14 '15 at 18:00
• @KevinMeredith - it's a little complicated, but the <?> will accept maps of any generic type, like String, Integer, etc... whereas <Object> will take only Object.... see this code, and try to compile it: pastebin.com/Ypj3exaP – rolfl Jul 14 '15 at 18:08