mkodekar / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

Maps.asMap(Object... varArgs) -> Map similar to Arrays.asList #1860

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
It would be awesome if guava had a method that takes variable object arguments.

Maps.asMap("key1", 1, "key2", "value2")  would yeild a hashmap, or immutable 
map.  

In json it would look something like this:
{'key1': 1, 'key2': 'value2'}

I don't think its too crazy to use variable arguments here.  Arrays.asList uses 
var args, why can't guava do this?

Original issue reported on code.google.com by joeheym...@gmail.com on 1 Oct 2014 at 8:06

GoogleCodeExporter commented 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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
> 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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 1 Oct 2014 at 8:40

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
tomas.za...@gmail.com, discouraging imagination stifles innovation and 
creativity

Original comment by joeheym...@gmail.com on 2 Oct 2014 at 4:21

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:07