Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A way to avoid redundant type parameters for ImmutableMap.builder() #883

Closed
gissuebot opened this issue Oct 31, 2014 · 8 comments
Closed

Comments

@gissuebot
Copy link

Original issue created by drothmaler on 2012-01-25 at 03:13 PM


When building immutalbe maps/lists you allways have the problem, that you will have respecify the element type, with the builder method.

So you end up writing things like:

public static final ImmutableMap<String, String> STRINGS = ImmutableMap.<String, String> builder()
        .put("A", "Foo")
        .put("B", "Foo")
        .put("C", "Foo")
        .build();

If you had a "builder" method, that takes the first value(-pair) you could write:

public static final ImmutableMap<String, String> STRINGS =
    ImmutableMap
        .builder("A", "Foo")
        .put("B", "Foo")
        .put("C", "Foo")
        .build();

It also could be named "put" instead.

@gissuebot
Copy link
Author

Original comment posted by joe.j.kearney on 2012-01-26 at 09:05 AM


If you wanted this to look the same, then you could use similar idea to CacheBuilder where the parameter types are fixed when build()ing or when adding a removalListener, rather than when creating the builder. Something like ImmutableMapGenericBuilder returned from the first put?

@gissuebot
Copy link
Author

Original comment posted by drothmaler on 2012-01-27 at 08:41 AM


Well, the CacheBuilder approach would be another option, but this would require a more major refactoring of the Immutable***.Builder classes. So I think a simple factory method with prototype-values, would be more light weight and more intuitive.
Also it is really simple to implement:

public static <K, V> ImmutableMap.Builder<K, V> builder(final K k, final V v) {
    return ImmutableMap.<K, V> builder().put(k, v);
}

that's all...

@gissuebot
Copy link
Author

Original comment posted by neveue on 2012-01-27 at 04:20 PM


I simply use ImmutableMap.of(K k1, V v1, K k2, V v2, K k3, V v3, K k4, V v4, K k5, V v5) and its overloads when I need less than 5 mappings. In the rare cases where I need more than 5 mappings, I use the Builder, accepting that I have to respecify the generics on the Builder.

I think that an overload accepting prototype values would complicate the API, for a small benefit, in the rare case where you need more than 5 mappings.

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2012-01-28 at 07:18 AM


In 2008, we diligently explored at least 15 different possible solutions (I'm not even exaggerating) for this problem. I hated those redundant type parameters. I was desperate to find a way to get rid of them. The debate stretched out over months. In the end, believe it or not, the last solution left standing happens to be exactly what you propose here, drothmaler. :-) Amazing.

But even though we more or less settled on this as the least-bad option, by the time we were left with it we just couldn't love it enough anymore to actually do it. Why?

  • I find the lack of symmetry disturbing. Remove the first entry? Gotta change the next "put" to "builder". Insert a new first entry? Make it "builder", then change the other "builder" to "put". I deeply loathe APIs and style conventions which cause simple focused changes to bleed onto logically unrelated lines. (I call it the blast radius.)
  • It's not that rare for the static types of k1 and v1 to be something slightly more specific than what the type of the map should be, so you have to upcast, making that .builder(k1, v1) line even more extra-specially gross.
  • But it's mostly about that symmetry thing.

Status: Triaged
Labels: Package-Collect

@gissuebot
Copy link
Author

Original comment posted by fry@google.com on 2012-02-16 at 07:18 PM


(No comment entered for this change.)


Status: Acknowledged

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2012-03-01 at 11:48 PM


I'm afraid we've decided that we're not going to add yet one more way to create an immutable map. I know the redundant type parameters are very depressing, believe me. But we have problems of one kind or another no matter what we do.


Status: WontFix

@gissuebot
Copy link
Author

Original comment posted by wasserman.louis on 2012-10-12 at 04:06 PM


Issue #1166 has been merged into this issue.

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2012-10-12 at 05:37 PM


Issue #1166 has been merged into this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant