[MDSAL-553] Add BindingMap to help with interfacing with keyed lists (Maps) Created: 11/May/20 Updated: 30/Sep/20 Resolved: 30/Sep/20 |
|
| Status: | Resolved |
| Project: | mdsal |
| Component/s: | Binding Spec |
| Affects Version/s: | None |
| Fix Version/s: | 7.0.0, 6.0.6 |
| Type: | Improvement | Priority: | Medium |
| Reporter: | Robert Varga | Assignee: | Ilya Igushev |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Description |
|
Current Builders allow setting properties only as an entire field. This is ends up being clunky, as we really want them to be built in a fluent way. This is especially problematic with Map-based properties, as populating the map requires the product to be present to extract the key: Map<Bar> bars = ...; Bar tmp = new BarBuilder().setName("one").build(); bars.put(tmp.key(), tmp); tmp = new BarBuilder().setName("two").build(); bars.put(tmp.key(), tmp); Foo foo = new FooBuilder().setBar(bars).build(); with the empty/null equivalence, we can improve this to something like: Foo foo = new FooBuilder() .addBar(new BarBuilder().setName("one").build()) .addBar(new BarBuilder().setName("two").build()) .build(); Since these methods are shifting backing structure maintenance to the builder, we also need to ensure proper encapsulation of copy mechanics.
|
| Comments |
| Comment by Robert Varga [ 24/Sep/20 ] |
|
Adding these methods will lead to significant increase of class size, as we will be creating these all over the place. The convenience used by people is that they construct a List and add them as they are built – and that is what we are essentially migrating from. The key problem we are facing is that extant Map-based utilities are not opinionated enough about their content. In our case, the Map is always in the form: Map<K extends Identifier<V>, V extends Identifiable<K>> i.e. we can mechanically derive the key from the entry, so each of our maps could have:
default void add(V value) {
put(value.key(), value);
}
Based on these observations, I think we would be best served with a custom Map factory/builder in vein of: public final class BindingMap { public static <K extends Identifier<V>, V extends Identifiable<K>> Map<K, V> of(V v1) { return Map.of(v1.key(), v1); } // ... plus all specializations to map to Map.of(), and then: public static <K extends Identifier<V>, V extends Identifiable<K>> Map<K, V> of(V... values) { // This, or something with Map.ofEntries() return Maps.uniqueIndex(Arrays.asList(values), Identifiable::key); } // and then a builder bridge public static <K extends Identifier<V>, V extends Identifiable<K>> BindingMap.Builder<K, V> builder(); public static <K extends Identifier<V>, V extends Identifiable<K>> BindingMap.Builder<K, V> builder(int expectedSize); public static final class Builder<K extends Identifier<V>, V extends Identifiable<K>> implements org.opendaylight.yangtools.concepts.Builder<Map<K, V>> { // ... and overall design of ImmutableMap.builder(), except we take advantage of Map.entryOf()/Map.ofEntries() // ... and instead of put()/putAll() we have ImmutableList.Builder-like: public Builder<K, V> add(V value); public Builder<K, V> addAll(V.. values); public Builder<K, V> addAll(Collection<V> values); } } With that we'll have a very convenient way of building Maps, hence we do not need add/remove and can just flow like this: Foo foo = new FooBuilder() .setBar(BindingMap.of( new BarBuilder().setName("one").build(), new BarBuilder().setName("two").build())) .build(); or builder-based: Foo foo = new FooBuilder() .setBar(BindingMap.<BarKey, Bar>buider() .add(new BarBuilder().setName("one").build()) .add(new BarBuilder().setName("two").build()) .build()) .build();
now ... the question is where to place such a class. It definitely needs to be in yang-binding.jar. Let's prototype it there first and see where it leads us. |