Feature: Provide collect methods instead of GenericType

[gpsfl]

Right now the most simple way to convert a Query to a typed Collection or Map is the following:

query.collectInto(new GenericType<Map<K, V>>() {});
query.collectInto(new GenericType<List<V>>() {});

This however has two disadvantages:

• It creates an anonymous inner class for each query
• It doesn't allow for the usage of Java's Collectors api

This is why I suggest adding the following two methods to handle the most common cases. From my understanding they could be implemented using the already existing logic from above:

query.collectToMap(K.class, V.class, Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue))
query.collectTo(V.class, Collectors.toList())

We're using JDBI in our production environment and I think this would save quite a lot of code lines / output classes while remaining as readable. If you agree with this change I can submit a pull request for it.

[hgschmie]

Hi @gpsfl,

thanks for opening an issue with the Jdbi project.

Use query.map(V.class).list() for query.collectInto(new GenericType<List<V>() {});, which is equivalent to the proposed query.collectTo(V.class, Collectors.toList()

I feel that the list collection is as succinct as a new collectTo method, which is basically implemented as mapTo(type).collect(collector)

• use query.map(V.class).collect(... collector ...) for any generic collector

For maps, you can use

Map<K, V> result = query.reduceRows(new LinkedHashMap<K, V>(), (map, rowView) -> {
    map.put(rowView.getRow(K.class), rowView.getRow(V.class));
    return map;
});

Map<K, V> map = query.mapTo(new GenericType<Map.Entry<K, V>>() {})
        .collect(Collectors.toMap(Map.Entry::getKey, Map::Entry::getValue));

If you are concerned about class counts / readability, use a constant:

private static final CUSTOM_MAP_TYPE = new GenericType<Map.Entry<KeyType, ValueType>>() {};
[...]
Map<KeyType, ValueType> map = query.mapTo(CUSTOM_MAP_TYPE).collect(toMap(Map.Entry::getKey, Map::Entry::getValue));

which has a similar amount of characters / type as

Map<KeyType, ValueType> map = query.collectToMap(KeyType.class, ValueType.class, toMap(Map.Entry::getKey, Map.Entry::getValue));

Please let me know if that works for you; we try to keep our API footprint small. I agree that our documentation could be better.

[gpsfl]

@hgschmie

I suggested the collectTo method because this "map & collect" pattern is one of the most common patterns I encountered when using jdbi. In my opinion it would be nice to have a convenience method for this, but I understand if you want to opt for a smaller api fingerprint instead.

Regarding the collectToMap method: Even if you're using constants for this, there are still plently of type combinations each creating an anonymous class. The proposed method could use type erasure to avoid any overhead while also avoiding unnecessary constants. Also it would reduce the amount of reflection necessary.

[hgschmie]

@stevenschlansker @sman-81 any thoughts?

[stevenschlansker]

For generic Collector use, it is easy enough to call query.map(DatabaseRow.class).stream().collect(myCollector)?

I don't quite see the compelling use case for List specialization: it seems that .map(type).list() does the same thing, or is there a case that doesn't cover this?

The Map special forms do feel like there could be improvement. The reduceRow solution works but might be common enough to warrant a ResultBearing api. Unfortunately the name map is used there in a totally different way, but you could imagine a ResultBearing.collectMap(Class<K>, Class<V>) (with a GenericType<T> overload)

[gpsfl]

@stevenschlansker
We're usually using immutable collections in our projects (for performance reasons & to avoid bugs). I also think this is what's considered best-practice nowadays. For this reason neither a list() nor a collectMap(Class<K>, Class<V>) would be of any use for us.

If you want to keep the .mapTo(MyClass.class).collect(Collectors.toMap(...)) pattern without a shortcut then perhaps a .mapToEntry(Key.class, Value.class) method would be a solution? Or to keep it more generic: .mapTo(Map.Entry.class, Key.class, Value.class).

[stevenschlansker]

@gpsfl I see, so you want to actually control the type of the List implementation.
Do you only use the imperative style code? Your proposal would allow you to collect(unmodifiableList) but wouldn't affect SqlObject return types like List<T>.

If we added the ability to customize the .list() method to use your own custom implementation, for example an ImmutableList collector, would that serve your use case well? Then in one place you could change the implementation of all collection actions, both for core and sqlobject.

Regarding Map, I was suggesting adding a shortcut, it would look like:

Map<K, V> result = handle.createQuery("select k, v from mapthing")
    .collectMap(K.class, V.class);

Since it is important to you that it be an ImmutableMap, we could also make the result container type customizable.

I am trying to avoid creating intermediate Map.Entry objects, since there is almost always a nicer way to write things without them.

[gpsfl]

@stevenschlansker
Yes, right now we're only using the imperative methods.

I think adding a collectMap method and the ability to customize the the collection implementations is a great idea. I assume you will configure this for the entire Jdbi object then, but would it also be possible to overwrite the implementation for a single query then? e.g.:

Map<K, V> result = handle.createQuery("select k, v from mapthing")
    .collectMap(K.class, V.class, LinkedHashMap::new);

Map<K, V> result = handle.createQuery("select k, v from mapthing")
    .mapTo(V.class)
    .list(LinkedList::new);

The only issue I see with this solution is that you still can't do some of the more complex things with the Collectors api. It's not as important (you can still do it with map + collect) but I wonder if you also have an idea how this could be done easier? Here's some common examples from our codebase:

.map(V.class).collect(toSet()) // Set
.map(V.class).colelct(toCollection(TreeSet::new)) // Custom collection
.map(V.class).collect(toMap(V::getId, Function.identity())) // Map by key
.map(V.class).collect(toMap(V::getId, Function.identity(), (a, b) -> a)) // Map by key (use first if not unique)
.map(V.class).collect(groupingBy(V::getGroupId)) // Grouping by
// (and all of the above with immutable collectors)

[stevenschlansker]

@gpsfl , I took a look at this and made some improvements. With #2303, it is now possible to configure the collector used for list() and a new set() returning method. This means you can register your ImmutableList etc once on Jdbi and it will be used everywhere you call list() (or return a List<V> via sqlobject).

I added a collectToMap helper which just is a facade for collect(toMap()). This is not as easy to parameterize the type, as we don't have a map-factory already and I didn't add one just for this feature yet.

I think the mapTo(V.class).list(LinkedList::new) syntax looks very appealing but it is not actually possible to implement this. mapTo(V.class) returns a type ResultIterable<V> which would then have a method <C> C list(Class<C> listType) but it is not possible to declare the static return type as C<V> - so any use of this api would only return the raw type LinkedList and lose the V parameter entirely. I don't think this is acceptable - so I think the best approach will remain Henning's suggestion of using collectInto(new GenericType<List<V>> (){}). If it is a problem have to make a class type for each unique type, you can alternately do collectInto(GenericTypes.parameterizeClass(List.class, V.class)).

Could you please take a look and see if my suggested improvements will help you out? I know it's not everything you asked for, but it should make some of the common cases easier.

[hgschmie]

This is addressed by #2331 and will be released as part of 3.38.0