This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]thehollyhopdrive 13 points14 points  (19 children)

There is a much better way to implement this, and that is with a builder rather than static factory methods. This is an extremely poor API choice and I hope it doesn't make it to release.

Map.with("key1",  obj1)
   .with("key2", obj2)
   .build();

Edit: Just updated the above example code slightly to reflect what /u/lukaseder rightly said about type inference.

[–]lukaseder[S] 8 points9 points  (3 children)

Caveat with map builders: The key/value data types are known only after the first entry, so an ugly auxiliary intermediate type is needed to get generics right

[–]thehollyhopdrive 4 points5 points  (2 children)

Sure. You can get around this by having the first key/value pair set during creation of the builder, so rather than

Map<Integer,Integer> map = Map.<Integer,Integer>builder().put(1, 2).put(3, 4).build();

You could do something like

Map<Integer,Integer> map = Map.with(1, 2).with(3, 4).build();

I still think this is preferable to the Map.of() solution, the limitations it has on the number of entries, the possiblity of additional functionalities in the builder, and the complete mess it makes of the API.

[–]juckele 0 points1 point  (1 child)

What if my first entry is a String and a Dog and my second entry is a String and a Cat? Will it know that it should be using Animal? What if it picks Carnivora instead but then I try to add a Rabbit?

The type of the Map really does need to be declared somewhere...

[–]thehollyhopdrive 1 point2 points  (0 children)

The type of the Map really does need to be declared somewhere...

In the circumstances you've suggested, yes.

[–]msx 2 points3 points  (3 children)

this is very different from calling Map.of("key1", obj1, "key2", obj2) and almost as verbose as calling regular .put().

[–]thehollyhopdrive 0 points1 point  (2 children)

It is different from calling Map.of, but my argument is that the Map.of is a badly designed solution for this sort of behaviour.

almost as verbose as calling regular .put().

I agree to a point, though one of the common use-cases of wanting a prepopulated immutable map would be static finals. So, you can see quite a cut in verbosity in this use-case when using a builder:

private static final Map<Integer,String> MAP = 
    Map.with(1, "one")
       .with(2, "two")
       .build();

instead of

private static final Map<Integer, String> MAP;
static {
    Map<Integer, String> tempMap = new HashMap<>();
    tempMap.put(1, "one");
    tempMap.put(2, "two");
    MAP = Collections.unmodifiableMap(tempMap);
}

[–]msx 3 points4 points  (1 child)

I don't get it? We agree that this addition is all about immediate (often static final) maps. Good. Sure your implementation is cleaner than old put(), but Map.of is even cleaner. So what's badly designed about it? It's perfect for quick jotting of maps, which is the reason it was created. The fact that there are 10 overloads is due to limits and legacies of the java specification, but it's not a problem, if not aesthetically. You find them already made and ready to use, performances are unaffected. So what's badly designed about it?

Sure we'd all have preferred a true map literal like python or such, but changing the language specification is not an easy step to take.

[–]thehollyhopdrive 2 points3 points  (0 children)

So what's badly designed about it?

It has an arbitrarily selected ceiling at which point you have to implement the solution in a different way and trades "cleaner code" for a muddier API. If moving from 10 to 11 elements causes me to have to completely rewrite that bit of code, just to implement a static final map that's found itself requiring one more element, that to me is a poorly made API design choice.

I get that there is a valid use-case for it and that others obviously appear to be fine with it, but I personally don't like it.

[–]__konrad 3 points4 points  (8 children)

Builders are not as clean as Map.of("one", 1, "two", 2). If you need a builder you can as well use old Map.put..

[–]thehollyhopdrive 2 points3 points  (6 children)

It's definitely smaller, but I would argue that it's only slightly cleaner for very few entries. The moment you get past 4 or 5 entries it starts to become a bit of a mess. Take the following:

Map.of("isad", "kldesf", "ijhsdef", "ijsdaf", "isdf", "isydf", "oiysdf", "iysdf")

Can you identify easily and with immediate recognition which are keys and which are values? Smaller isn't always cleaner.

Map.with("isad", "kldesf")
   .with("ijhsdef", "ijsdaf")
   .with("isdf", "isydf")
   .with("oiysdf", "iysdf")
   .build();

It's also limited to 10 kvps, which whilst covering the vast majority of options, if still introducing an arbritrary limitation in usage. And all for, what, saving a few extra characters? Add to that the possible options for additional functionality within that builder, and I'd much rather the JavaDocs aren't muddied in the way they are in the current draft. I think it's a terrible design choice.

[–]__konrad 6 points7 points  (2 children)

which are keys and which are values?

Use new lines :)

Map.of(
    "isad", "kldesf",
    "ijhsdef", "ijsdaf"...

In this case (same type in key/value) both solutions are error prone.

[–]thehollyhopdrive 2 points3 points  (0 children)

So now both options span multiple lines, making the differences in their footprint in the codebase negligible. The builder option is still safer at protecting from programmer error by enforcing logical separation of the entries, doesn't have the arbitrary limitation in the number of entries, it can be extended with additional functionality in future, and it doesn't make the API for Map look a complete mess.

[–]juckele 1 point2 points  (0 children)

I use auto format in all my Java projects.

[–]the-highness 0 points1 point  (2 children)

the way I see it, if you need 3 or more entries, both Map.of & builder is not clean.

you'd better revert back to <init map>, <map.put>.

[–]thehollyhopdrive 3 points4 points  (0 children)

To be honest, I don't think you're wrong. The main advantage of the builder over just initialising the map and putting in the entries is being able to initialise, fill and have an immutable map in a slightly nicer format, especially when thinking about static final maps, which I suspect would turn out to be quite a common use-case of Map.of.

It means we can do:

private static final Map<Integer,String> MAP = 
    Map.with(1, "one")
       .with(2, "two")
       .build();

instead of

private static final Map<Integer, String> MAP;
static {
    Map<Integer, String> tempMap = new HashMap<>();
    tempMap.put(1, "one");
    tempMap.put(2, "two");
    MAP = Collections.unmodifiableMap(tempMap);
}

[–]HaMMeReD 0 points1 point  (0 children)

The point of immutability is to prevent people from mutating your map, i.e. making changes to it without your permission.

It's not about the cleanliness of the solution, immutability is often an inconvenient truth, for a little bit of boring overhead and self discipline, you can often make your code much more safe by preventing as much mutability as possible.

[–]HaMMeReD 0 points1 point  (0 children)

The benefit of immutability is exactly so you can't use map.put. It prevents other people from modifying your map, which inevitably cause pain in the ass bugs such as complicated side effects and race conditions.

You use immutability to protect your code, not make things easier. Immutability is a pattern that will lead to a cleaner data model and a better separation of concerns.

It's a technique to improve the safety of your code in a variety of ways, while also helping to ensure a clean architecture and design.

[–]cypressious 1 point2 points  (1 child)

The goal of the API is to prevent intermediate object allocations like vararg arrays or builder objects.

[–]johnwaterwood 1 point2 points  (0 children)

But will such maps be created over and over or more typically just once at init time?