Closed GoogleCodeExporter closed 9 years ago
You're losing all the available type safety here: you can't have a distinct
key/value type, and your example actually illustrates that you're no longer
enforcing types on the values.
What's wrong with ImmutableMap.builder().put(key1, value1).put(key2,
value2).build()?
Original comment by lowas...@google.com
on 1 Oct 2014 at 8:07
Code sample for a map of string to object:
public static Map<String, Object> asMap(Object... args) {
Map<String, Object> argMap = new HashMap<String, Object>();
for (int i = 0; i < args.length; i += 2) {
String key;
try {
key = (String) args[i];
} catch (ClassCastException cce) {
System.err.println(cce.getMessage());
System.err.println("args[" + i + "] " + args[i].toString());
throw cce;
}
if (i + 1 < args.length) {
Object value = args[i + 1];
argMap.put(key, value);
}
}
return argMap;
}
It doesn't necessarily have to be string -> object, we could just write a
generic Map<Object,Object> function.
Original comment by joeheym...@gmail.com
on 1 Oct 2014 at 8:09
I'm proposing a non-builder approach.
ArrayList.asList is not a builder.
We don't have
List<String> foo =
ImmutableList.builder().add("foo").add("bar").add("baz").build();
List<String> foo = Arrays.asList("foo", "bar", "baz");
Tell me which one is cleaner?
Original comment by joeheym...@gmail.com
on 1 Oct 2014 at 8:11
Arrays.asList doesn't have to give up type safety, but this would.
Additionally, we do provide ImmutableMap.of() for small numbers of entries.
Would that help for your use case?
Original comment by lowas...@google.com
on 1 Oct 2014 at 8:13
[deleted comment]
ImmutableMap.of doesn't work with variable arguments.
If I have a funcion:
void foo(Object ... args) {
ImmutableMap.of(args) // doesn't work
}
Original comment by joeheym...@gmail.com
on 1 Oct 2014 at 8:16
That's because your foo function is losing the type safety, too. If you have
arguments that are being passed around in pairs, they should probably be passed
with ImmutableMap.of to start with. Consider changing the arguments of your
foo function instead?
Original comment by lowas...@google.com
on 1 Oct 2014 at 8:18
Not just losing type safety, losing safety in even the number of arguments.
Original comment by cgdecker@google.com
on 1 Oct 2014 at 8:22
Sometimes type safety isn't that big of a deal. Lets say you wanted to write a
test with a dummy map? You know the type of the arguments because you
explicitly put them in the test.
Either way, I'm sure you could make it so types are enforced in the pairs of
arguments with generic types.
Original comment by joeheym...@gmail.com
on 1 Oct 2014 at 8:30
> Either way, I'm sure you could make it so types are enforced in the pairs of
arguments with generic types.
Not...really? There's no good way to enforce generics even at runtime when you
would be getting nested generics, e.g. Map<Key, Future<Value>>. And in
practice, essentially all users would have to cast the resulting map -- there's
not much actual use for a Map<Object, Object>, and most users' APIs won't
accept anything of the sort.
As far as tests go, I'd definitely expect the majority of test maps to be quite
small, small enough that they can use the ImmutableMap.of factories. And for
larger maps, there's a reasonable, type-safe alternative that's only slightly
longer, via the ImmutableMap.Builder.
Original comment by lowas...@google.com
on 1 Oct 2014 at 8:34
New idea:
What if Maps.asMap was a wrapper to the builder pattern?
Original comment by joeheym...@gmail.com
on 1 Oct 2014 at 8:36
How exactly would that work differently from your previous suggestions? The
API would still have to take Object..., wouldn't have type safety, and wouldn't
enforce that there was an even number of arguments.
Original comment by lowas...@google.com
on 1 Oct 2014 at 8:37
I'm going to go ahead and just say this isn't something we'll be doing. There's
little to be gained by writing
Maps.asMap(
a, b,
c, d,
e, f);
instead of
ImmutableMap.of(
a, b,
c, d,
e, f);
or even (if you have lots of entries)
ImmutableMap.builder()
.put(a, b)
.put(c, d)
.put(e, f)
// ...
.build();
and lots of disadvantages. It's also not really similar to Arrays.asList, which
is a lightweight view of an array, not something new built from the contents of
an array.
Original comment by cgdecker@google.com
on 1 Oct 2014 at 8:40
Original comment by cgdecker@google.com
on 1 Oct 2014 at 8:40
Yeah I figured you were going to reject this. Thanks for listening to my
stupid idea.
Original comment by joeheym...@gmail.com
on 1 Oct 2014 at 8:44
Thanks for the suggestion. In this case, I just think that the APIs we
currently provide for this cover the use cases such a method would have
sufficiently well while preserving type safety, which is valuable.
Original comment by cgdecker@google.com
on 1 Oct 2014 at 8:58
joeheym..., FYI just academic case for completeness and linkage to related
issue:
You could reach your goal with recently added feature:
https://code.google.com/p/guava-libraries/issues/detail?id=320 , which makes
possible to call
ImmutableMap.copyOf(ImmutableList.of(Maps.immutableEntry("key1",
1),Maps.immutableEntry("key2", "value2")))
(working out from my head, probably some explicit generic type inference would
be necessary)
Anyway I cannot imagine why one would write it this way and really discourage
it, the ImmutableMap.builder (or constructor for 5- entries) is certainly the
best approach.
Original comment by tomas.za...@gmail.com
on 2 Oct 2014 at 6:40
tomas.za...@gmail.com, discouraging imagination stifles innovation and
creativity
Original comment by joeheym...@gmail.com
on 2 Oct 2014 at 4:21
Well, anyway, this was the best I could come up with to guarantee the pairs and
type safety:
@SafeVarargs
public static <K,V> Map<K, V> asMap(Pair<K, V>... entries) {
Map<K, V> argMap = new HashMap<K, V>();
for(Pair<K,V> entry : entries) {
argMap.put(entry.k, entry.v);
}
return argMap;
}
public static class Pair<K,V> {
public K k;
public V v;
Pair(K key, V value) {
this.k = key; this.v = value;
}
}
public static <K,V> Pair<K,V> p(K key, V value) {
return new Pair<K, V>(key, value);
}
Then:
Map<String,Integer> pairMap = asMap(p("a",1), p("b",2));
System.out.println(pairMap);
Yields:
{b=2, a=1}
Original comment by joeheym...@gmail.com
on 3 Oct 2014 at 6:59
This issue has been migrated to GitHub.
It can be found at https://github.com/google/guava/issues/<issue id>
Original comment by cgdecker@google.com
on 1 Nov 2014 at 4:08
Original comment by cgdecker@google.com
on 3 Nov 2014 at 9:07
Original issue reported on code.google.com by
joeheym...@gmail.com
on 1 Oct 2014 at 8:06