vavr-io / vavr

vʌvr (formerly called Javaslang) is a non-commercial, non-profit object-functional library that runs with Java 8+. It aims to reduce the lines of code and increase code quality.
https://vavr.io
Other
5.73k stars 637 forks source link

Add concat methods to Tuple classes #2215

Closed jlorenzen closed 6 years ago

jlorenzen commented 6 years ago

It would be handy if the individual Tuple classes had concat or append methods to them. I've ran into two instances lately where they would have been helpful. The jOOL Tuple classes have them. I would use them but the jOOQ Tuple classes don't have the apply method which I've come to love in the vavr Tuple classes. And I'm trying to standardize on using just vavr.

nfekete commented 6 years ago

@jlorenzen do you mean widening and narrowing Tuples from the right?

jlorenzen commented 6 years ago

@nfekete Yeah I think so. Basically the following:

final Tuple2<String, String> user = Tuple.of("John", "Doe");
final Tuple3<String, String, String> userWithEmail = user.concat("john.doe@example.com");
jlorenzen commented 6 years ago

@nfekete @danieldietrich I would be interested in making these changes if you guys would welcome them and offer any suggestions?

ruslansennov commented 6 years ago

@jlorenzen The append and prepend methods were added and removed twice. It was a difficult decision, see the last discussion.

jlorenzen commented 6 years ago

@ruslansennov That last discussion didn't really go deep into why it was removed. There is hint of people not liking that Tuple8 (the last numbered Tuple) had to throw an exception since there isn't a Tuple9 class. Looking at the last jOOL Tuple class, Tuple16, they just don't add a concat method. That would seem like a reasonable compromise. Here is the reason I'd like to see the vavr Tuple classes have at least a concat or append method. Lately I've been using the Validation class to perform validation on HTTP request parameters. I then convert those to a Tuple to be used in the next step. So something like the following:

private SignInResult signIn(final String email, final String password) {
    validate(email, password)
        .flatMap(emailAndPassword -> emailAndPassword.apply(signInService::signIn));
}

private Either<Throwable, Tuple2<Email, Password> validate(final String email, final String password) {
    // create validation objects for each param
    return Validation.combine(emailValidation, passwordValidation)
        .ap(Tuple::of)
        .toEither();
}

Given my example above, my problem is the signInService.signIn method takes a third parameter that doesn't come from client input. Perhaps it might include the IP Address of the client. So here is what I would like to do:

private SignInResult signIn(final String email, final String password) {
    validate(email, password)
        .map(emailAndPassword -> emailAndPassword.append(getClientAddress()))
        .flatMap(signInValues -> signInValues.apply(signInService::signIn));
}

Instead I have to reference the tuple values (which I can't stand to do because I feel it hurts readability):

private SignInResult signIn(final String email, final String password) {
    validate(email, password)
        .flatMap(emailAndPassword -> signInService.signIn(emailAndPassword._1, emailAndPassword._2, getClientAddress()));
}

I really enjoy using the Tuple.apply methods because it helps me avoid referencing the individual values using ._n.

danieldietrich commented 6 years ago

@jlorenzen Yes, I understand the benefits of having it. So, let's add the concat method.

(Having an append might ignite the desire to also add a prepend, which is unnecessary when swapping the tuples and using concat instead.)

Does anyone see a use case for the following?

jlorenzen commented 6 years ago

@danieldietrich Hey I took an initial stab at making these changes and I would appreciate some feedback before I go any further. See the PR here: https://github.com/vavr-io/vavr/pull/2221. Thanks!

danieldietrich commented 6 years ago

Fixed with #2221