RFC: draft API for JEP 269 Convenience Collection Factories

[Ivan Gerasimov]

Thank you Stuart for yours comments!

On 17.10.2015 20:23, Stuart Marks wrote:
> On 10/14/15 5:56 AM, Ivan Gerasimov wrote:
>>          Map<Integer,Character> m1 = MyCollections.<Integer,Character>
>>                   ofKeys( 1,   2,   3,   4,   5)
>>                  .vals(  'a', 'b', 'c', 'd', 'e');
>
> Yes, we considered a bunch of different alternatives.
>
> It looks like you're wrestling a bit with type inference :-), given 
> that you had to invoke this with a type witness.

Yes, I should have been more careful with the types :-)
The KeyHolder class, returned by ofKeys should not depend on the V type. 
Then the inference would work as expected.

I totally agree with your point on that it is more preferable to the key 
and corresponding value next to each other.
But what if both are provided, so the user can choose what's more 
appropriate?
Would you take a look at another sample code?

First, it's more accurate with type inference.
Second, it allows filling a map either as specifying separate entries, 
or specifying an array of keys and another array of values.

--------------------------------------------
import java.util.*;
import java.util.function.*;

public class MB {

     public static void main(String[] a) {
         Map<Integer,Character> m1 = MapBuilder
             .k(1).v('a')
             .k(2).v('b')
             .k(3).v('c')
             .k(4).v('d')
             .k(5).v('e')
             .k( 6,   7,   8)
             .v('f', 'g', 'h')
             .build();
         m1.forEach((k, v) -> System.out.println("m1[" + k + "] = " + v));

         Map<Integer,Character> m2 = MapBuilder
             .add(101, 'A')
             .add(102, 'B')
             .add(103, 'C')
             .add(104, 'D')
             .build();
         m2.forEach((k, v) -> System.out.println("m2[" + k + "] = " + v));
     }
}


class MapBuilder {
     public static <K> KeysHolder<K> k(K... keys) {
         return new KeysHolder<>(keys);
     }

     public static <K,V> KeysValsHolder<K,V> add(K key, V val) {
         return k(key).v(val);
     }

     public static class KeysHolder <K> {
         K[] keys;
         KeysHolder(K... keys) {
             this.keys = keys;
         }

         public <V> KeysValsHolder<K,V> v(V... vals) {
             return new KeysValsHolder<>(keys, vals);
         }
     }

     public static class KeysValsHolder <K,V> {
         K[] keys;
         V[] vals;

         KeysValsHolder(K[] keys, V[] vals) {
             this.keys = keys;
             this.vals = vals;
         }

         public KeysValsHolder<K,V> k(K... keys) {
             int len0 = this.keys.length;
             int len1 = keys.length;
             this.keys = Arrays.copyOf(this.keys, len0 + len1);
             System.arraycopy(keys, 0, this.keys, len0, len1);
             return this;
         }

         public KeysValsHolder<K,V> v(V... vals) {
             int len0 = this.vals.length;
             int len1 = vals.length;
             this.vals = Arrays.copyOf(this.vals, len0 + len1);
             System.arraycopy(vals, 0, this.vals, len0, len1);
             return this;
         }

         public KeysValsHolder<K,V> add(K key, V val) {
             return k(key).v(val);
         }

         Map<K,V> build() {
             int len = keys.length;
             if (vals.length != len) {
                 throw new IllegalArgumentException();
             }
             if (len == 0) {
                 return Collections.<K,V>emptyMap();
             } else if (len == 1) {
                 return Collections.<K,V>singletonMap(keys[0], vals[0]);
//            } else if (len < 33) {
//                return new UnsortedArrayBasedMap(keys, vals);
//            } else if (len < 1025) {
//                return new SortedArrayBasedMap(keys, vals);
             } else {
                 Map<K,V> map = new HashMap<>();
                 for (int i = 0; i < len; ++i) {
                     map.put(keys[i], vals[i]);
                 }
                 return Collections.unmodifiableMap(map);
             }
         }
     }
}
--------------------------------------------

Sincerely yours,
Ivan


> The issue here is that the target type doesn't back-propagate through 
> the chained method call .vals() to the call of .ofKeys(). 
> Incidentally, a similar problem occurs in the "obvious" builder 
> approach of doing something like this:
>
>     Map<Integer,Character> map = Map.builder()
>        .add(1, 'a')
>        .add(2, 'b')
>        ....
>        .build();
>
> It's possible to pursue workarounds for this problem, but then you're 
> heading down the rabbit hole....
>
> A quasi-syntactic issue is that it's preferable to have each key and 
> value next to each other, so that each pair can be on its own line. 
> This makes it easier to maintain. If all the keys are listed together 
> first, followed by all the values, the pairing can only be made to 
> work if they're all short and if there are few enough pairs (as in 
> your example). Things start to break down if the keys/values 
> themselves are long, or if there are a lot of them, pushing the keys 
> and values lists onto multiple lines. Consider my example from the 
> head of the thread:
>
>     Map<String,TypeUse> m = Map.ofEntries(
>         entry("CDATA",       CBuiltinLeafInfo.NORMALIZED_STRING),
>         entry("ENTITY",      CBuiltinLeafInfo.TOKEN),
>         entry("ENTITIES", CBuiltinLeafInfo.STRING.makeCollection()),
>         entry("ENUMERATION", CBuiltinLeafInfo.STRING.makeCollection()),
>         entry("NMTOKEN",     CBuiltinLeafInfo.TOKEN),
>         entry("NMTOKENS", CBuiltinLeafInfo.STRING.makeCollection()),
>         entry("ID",          CBuiltinLeafInfo.ID),
>         entry("IDREF",       CBuiltinLeafInfo.IDREF),
>         entry("IDREFS",
> TypeUseFactory.makeCollection(CBuiltinLeafInfo.IDREF));
>         entry("ENUMERATION", CBuiltinLeafInfo.TOKEN));
>
> Maintaining the association between keys and values is challenge. 
> Here's my attempt, where keys and values are associated by indentation 
> level:
>
>     Map<String,TypeUse> m = Map.<String,TypeUse>
>          ofKeys("CDATA",
>                   "ENTITY",
>                     "ENTITIES",
>                       "ENUMERATION",
>                         "NMTOKEN",
>                           "NMTOKENS",
>                             "ID",
>                               "IDREF",
>                                 "IDREFS",
>                                   "ENUMERATION")
>         .vals(  CBuiltinLeafInfo.NORMALIZED_STRING,
>                   CBuiltinLeafInfo.TOKEN,
>                     CBuiltinLeafInfo.STRING.makeCollection(),
>                       CBuiltinLeafInfo.STRING.makeCollection(),
>                         CBuiltinLeafInfo.TOKEN,
> CBuiltinLeafInfo.STRING.makeCollection(),
>                             CBuiltinLeafInfo.ID,
>                               CBuiltinLeafInfo.IDREF),
>
> TypeUseFactory.makeCollection(CBuiltinLeafInfo.IDREF),
>                                   CBuiltinLeafInfo.TOKEN);
>
> With more pairs, the distance between each key and its corresponding 
> value also increases. You could try to reduce the vertical distance by 
> filling the lines (more keys/values per line) but that makes the 
> association challenges even harder. Finally, consider the pain of 
> inserting or removing a key/value pair from the middle.
>
> s'marks
>