jOOQ / jOOL

jOOλ - The Missing Parts in Java 8 jOOλ improves the JDK libraries in areas where the Expert Group's focus was elsewhere. It adds tuple support, function support, and a lot of additional functionality around sequential Streams. The JDK 8's main efforts (default methods, lambdas, and the Stream API) were focused around maintaining backwards compatibility and implementing a functional API for parallelism.
http://www.jooq.org/products
Apache License 2.0
2.09k stars 168 forks source link

Add Tuple.toMap() methods #192

Closed bartolom closed 8 years ago

bartolom commented 8 years ago

I'm not sure if this can already be acomplished in some other way with JOOL, so you might hint me there.

I'll try to outline my use case quickly, so the need becomes a bit clearer, and you can better judge if this is something JOOL should support.

In have some Spring MVC controllers and they do what they typically do in Spring: they call services and render the result as JSON with Jackson. I now have some cases where the service layer is returing me a List and I want to correlate it with some other service. I have my Jackson stuff already in place (so I render bigger objects then in below example). I just need a simple anonymous wrapper so I can pass it to Jackson and I thought that JOOL could do the job for me.

JOOL and Jackson do render in 0.9.10

0: {
 "v1": "someValue0",
 "v2": "someCorrelatedValue0",
 "v3": "more"
},
1: {
 "v1": "someValue1",
 "v2": "someCorrelatedValue1",
 "v3": "more"
},

What I was looking for is something more semantic where "v1" and "v2" could be replaced, so I can use it as an API

0: {
 "someNameA": "someValue0",
 "someNameB": "someCorrelatedValue0",
 "someNameC": "more"
},
1: {
 "someNameA": "someValue1",
 "someNameB": "someCorrelatedValue1",
 "someNameC": "more"
},

I can achieve this with some additional helper methods written by myself but not in a very generalized way. I just thought it might be nice to have this out of the box from a library like JOOL

something along the lines:

seq(list)
  // other stream/seq calls
  .map(x -> tuple(x, someService.foo(x), "z")
  // other stream/seq calls
 .map(t -> SomeJoolUtil.toNamedMap(t, "someNameA", "someNameB", "someNameC"))
 .toList();

Of course JOOL could be strict and throw an exception, when the count of the names do not match to count of the values in the tuple.

Because the Json output should be deterministic I have my current helper method with a TreeMap::new but I think this could be made similar to JDK 8 where I can pass a mapSupplier

java.util.stream.Collectors.toMap(Function<? super T, ? extends K>, Function<? super T, ? extends U>, BinaryOperator<U>, Supplier<M>)

btw: My helper methods do currently look like this. Please ignore the use of NavigableMap, I just happen to like it, as it is nice for unit testing.

List<Item> list = serviceA.foo();
List<NavigableMap<String, Object>> list2 = seq(list)
  .map(i -> tupleMap(
    tuple("someNameA", i))),
    tuple("someNameB", serviceB.correlated(i.getEmbeddedId())),
    tuple("someNameC", "more")
    .toList();

// [...]

@SafeVarargs
private static NavigableMap<String, Object> tupleMap(Tuple2<String, Object>... t) {
  return Arrays.stream(t).collect(toMap(
    k -> k.v1(),
    v -> v.v2(),
    throwingMerger(),
    TreeMap::new));
}

private static <T> BinaryOperator<T> throwingMerger() {
    return (u, v) -> {
        throw new IllegalStateException(
                String.format("Duplicate key for values '%s' and '%s'", u, v));
    };
}

I don't know if "toNamedMap()" is the perfect method name. Maybe something like "toSemanticMap()" would be even better.

bartolom commented 8 years ago

It seems that JDK9 will have related stuff to my todays helper method in http://openjdk.java.net/jeps/269

Map.ofEntries(
    entry(k1, v1),
    entry(k2, v2),
    entry(k3, v3),
    // ...
    entry(kn, vn));
lukaseder commented 8 years ago

Hmm, interesting ideas. So... Let's add:

interface Tuple {
    // { "v1" : v1, "v2" : v2, "v3" : v3, ... }
    Map<String, ?> toMap();

    // { keyMapper(0) : v1, keyMapper(1) : v2, ... }
    <K> Map<K, ?> toMap(Function<? super Integer, ? extends K> keyMapper);
}

// and then
interface Tuple[N]<T1, T2, ..., TN> {
    <K> Map<K, ?> toMap(Supplier<? extends K> key1, Supplier<? extends K> key2, ...);
}

Is that along the lines of what you inteded?

bartolom commented 8 years ago

Obviously you are way more versed with generics then me, so I might misinterpret your above suggestion.

interface Tuple {
    // { "v1" : v1, "v2" : v2, "v3" : v3, ... }
    Map<String, ?> map();

As of JDK8 the method ".map()" is now predominantly used in Stream/Seq for in the meaning of map/reduce. So I would be prefer a .toMap() or .asMap() (or ofTuple() when the other way around) but you might have a broader use in mind.

// { keyMapper(0) : v1, keyMapper(1) : v2, ... }
    <K> Map<K, ?> map(Function<? super Integer, ? extends K> keyMapper);

I must admit that I don't get this one interface method. This one would be used by the library internally or from a end user as well? Is this a alternative way, where I could do something like when I have a tuple outside of a stream/seq?

So if I get your proposal it would be for my use case something like:

seq(list)
  // other stream/seq calls
  .map(x -> tuple(x, someService.foo(x), "z")
  // other stream/seq calls
 .map(t -> t.map(
     () -> "someNameA", 
     () -> "someNameB", 
     () -> "someNameC"))
 .toList();

So the use of Supplier would make it very flexible, my use case with String is just a very basic one. I can see the potential here.

lukaseder commented 8 years ago

As of JDK8 the method ".map()" is now predominantly used in Stream/Seq for in the meaning of map/reduce

Yes, I agree. toMap() is better. I suspect that array() and others should also be renamed to toArray(), then...

I must admit that I don't get this one interface method. This one would be used by the library internally or from a end user as well? Is this a alternative way, where I could do something like when I have a tuple outside of a stream/seq?

It's the generic toMap() method, which doesn't depend on the degree of the tuple, but takes the degree as an argument to the function that provides the map key.

So if I get your proposal it would be for my use case something like ...

Exactly. Of course, a variant that directly takes strings as an overload, instead of Supplier is feasible as well.

bartolom commented 8 years ago

I looked over to the javaslang projects an they also use a toXxx() pattern. For instance https://github.com/javaslang/javaslang/blob/master/javaslang/src-gen/main/java/javaslang/Tuple.java#L34 has a toSeq(). The JDK has also the Arrays.asList() but I would prefer the toMap() as well.

The convenience with the overloaded version with String would be nice because it would cover a common use case like mine.

bartolom commented 8 years ago

As mentioned above I would like to supply the Map implementation, to have predicitable results with a TreeMap

In my "varargs-thinking" this would be something similar to provide a BinaryOperator<U> and a Supplier<M> like in JDK8:

seq(list)
  .map(x -> tuple(x, someService.foo(x), "z")
  .map(t -> t.toMap(
     (u, v) -> u,
     TreeMap::new,
     () -> "someNameA", 
     () -> "someNameB", 
     () -> "someNameC"))
  .toList();

I think this signature would be a bit more end user friendly. As I state the "complex" stuff at the beginning and add repetetive stuff after this. So adding yet another row is easy, and my IDE code completion would show the BinaryOperator and the map supplier at the beginning.

But as I understand org.jooq.lambda.tuple.Tuple[N] are generated source code so in theory the BinaryOperator<U> and Supplier<M> could be at the end as well.

seq(list)
  .map(x -> tuple(x, someService.foo(x), "z")
  .map(t -> t.toMap(
     () -> "someNameA", 
     () -> "someNameB", 
     () -> "someNameC"),
     (u, v) -> u,
     TreeMap::new)
  .toList();

This signature would be closer to java.util.stream.Collectors.toMap(Function<? super T, ? extends K>, Function<? super T, ? extends U>, BinaryOperator<U>, Supplier<M>) Adding rows/values would happen in the middle and the signature would be shifting. So it would potentially be harder to debug with current JDK8 support in IDEs as mismatches with functional interfaces are hard to spot.

lukaseder commented 8 years ago

Hmm, interesting. I would have expected a toMap() method to return a LinkedHashMap, in order to preserve tuple ordering, rather than to introduce any natural order.

I think this signature would be a bit more end user friendly

I dare to disagree :) It is rather complex. It's always a good idea to avoid unnecessary arguments. But we might well factor out the essential idea in your suggestions. To add a new methods:

interface Tuple {
    <R, A> R collect(Collector<?, A, R> collector);
    <R> R collect(Supplier<R> supplier,
                  BiConsumer<R, ?> accumulator,
                  BiConsumer<R, R> combiner);
}

This way, you could re-use your existing Collectors.toMap() collector with all the signature complexity. I personally don't see all of these Collectors.toMap() overloads to be first class citizens on a Tuple type... What do you think?

bartolom commented 8 years ago

"Add <R, A> R Tuple.collect(Collector<?, A, R>) #193" sounds like a clean separation of the Tuple world with their order, and less frequent use case like my properbly is.

Thank you for the quick implementatiom. I will give the master branch a try later and. looking forward to the release

lukaseder commented 8 years ago

Indeed, but unfortunately, I'm afraid that this Tuple.collect() method is not possible, because the <T> type of the Collector cannot be specified in Java. It would have to be <T super T1 | T2 | ... | TN> for Tuple[N]<T1, T2, ..., TN>.

Looking forward to your feedback!