Either

Landei · 2. Juni 2014 um 02:23

Vorschlag für Either ohne Laziness und mit Optional-inspirierter Benamsung:

import java.util.NoSuchElementException;
import java.util.Optional;
import java.util.function.Function;
import java.util.function.Supplier;

public abstract class Either<A, B> {

    Either() {
    }

    public static <A, B> Left<A, B> leftOf(A a) {
        return new Left<>(a);
    }

    public static <A, B> Right<A, B> rightOf(B b) {
        return new Right<>(b);
    }

    public abstract boolean isLeft();

    public abstract boolean isRight();

    public abstract A left() throws NoSuchElementException;

    public abstract B right() throws NoSuchElementException;

    public abstract A leftOrElse(A defaultValue);

    public abstract B rightOrElse(B defaultValue);

    public abstract A leftOrElseGet(Supplier<A> defaultSupplier);

    public abstract B rightOrElseGet(Supplier<B> defaultSupplier);

    public abstract Optional<A> leftOpt();

    public abstract Optional<B> rightOpt();

    public abstract <A1> Either<A1, B> mapLeft(Function<? super A, ? extends A1> fn);

    public abstract <B1> Either<A, B1> mapRight(Function<? super B, ? extends B1> fn);

    public abstract <A1, B1> Either<A1, B1> bimap(Function<? super A, ? extends A1> fnA, Function<? super B, ? extends B1> fnB);

    public abstract <C> C either(Function<? super A, ? extends C> fnA, Function<? super B, ? extends C> fnB);

    public abstract Either<B, A> swap();
}

import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Function;
import java.util.function.Supplier;

public final class Left<A, B> extends Either<A, B> {

    private final A a;

    public Left(A a) {
        Objects.requireNonNull(a);
        this.a = a;
    }

    @Override
    public boolean isLeft() {
        return true;
    }

    @Override
    public boolean isRight() {
        return false;
    }

    @Override
    public A left() {
        return a;
    }

    @Override
    public B right() throws NoSuchElementException {
        throw new NoSuchElementException("right() call on Left");
    }

    @Override
    public A leftOrElse(A defaultValue) {
        return left();
    }

    @Override
    public B rightOrElse(B defaultValue) {
        return defaultValue;
    }

    @Override
    public A leftOrElseGet(Supplier<A> defaultSupplier){
        return left();
    }

    @Override
    public B rightOrElseGet(Supplier<B> defaultSupplier) {
        return defaultSupplier.get();
    }

    @Override
    public Optional<A> leftOpt() {
        return Optional.of(left());
    }

    @Override
    public Optional<B> rightOpt() {
        return Optional.empty();
    }

    @Override
    public <A1> Either<A1, B> mapLeft(Function<? super A, ? extends A1> fn) {
        return leftOf(fn.apply(left()));
    }

    @Override
    public <B1> Either<A, B1> mapRight(Function<? super B, ? extends B1> fn) {
        return leftOf(left());
    }

    @Override
    public <A1, B1> Either<A1, B1> bimap(Function<? super A, ? extends A1> fnA, Function<? super B, ? extends B1> fnB) {
        return leftOf(fnA.apply(left()));
    }

    @Override
    public <C> C either(Function<? super A, ? extends C> fnA, Function<? super B, ? extends C> fnB) {
        return fnA.apply(left());
    }

    @Override
    public Either<B, A> swap() {
        return rightOf(left());
    }

    @Override
    public String toString() {
        return String.format("Left(%s)", left());
    }

    @Override
    public int hashCode() {
        return 29 * left().hashCode();
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj) {
            return true;
        }
        if (obj instanceof Left) {
            Left<?,?> that = (Left) obj;
            return this.left().equals(that.left());
        }
        return false;
    }
}

Right natürlich analog.

Ich finde die Implementierung so recht vernünftig und vor allem ohne große Überraschungen. leftOpt und rightOpt ist ein Kompromiss, weil ich bei Listen dann auch gerne headOpt und tailOpt hätte.

Ob wir aus Performancegründen bei mapLeft und mapRight casten statt neu zu wrappen, können wir uns noch überlegen.

cmrudolph · 2. Juni 2014 um 04:45

Den Konstruktor würde ich so machen:

    Objects.requireNonNull(a);
    this.a = a;
}```
Kürzer könnte man natürlich noch schreiben ```this.a = Objects.requireNonNull(a);``` das halte ich aber für nicht ganz so leserlich.

Die Sichtbarkeit des Konstruktors würde ich noch auf package private reduzieren.

Habe mir den Code aber noch nicht im Detail angesehen, das hole ich heute Abend nach.

Auf lazy-Initialize würde ich auch verzichten, weil die nonnull-Eigenschaft dabei nicht geprüft werden kann.

Bei mapLeft und mapRight würde ich casten. Allerdings nicht unbedingt aus Performancegründen, sondern um keine weiteren, überflüssigen Objekte zu erzeugen.

Landei · 2. Juni 2014 um 06:12

[QUOTE=cmrudolph]Den Konstruktor würde ich so machen:

    Objects.requireNonNull(a);
    this.a = a;
}```
[/quote]

Das compiliert nicht. Und für das `a` kann man auch jetzt schon einen Untertyp übergeben, ohne irgendwelche Klimmzüge.


> 
> Kürzer könnte man natürlich noch schreiben ```this.a = Objects.requireNonNull(a);``` das halte ich aber für nicht ganz so leserlich.


Habe ich noch gar nicht gesehen, dass das einen Rückgabewert hat, können wir ruhig so machen.


> 
> Die Sichtbarkeit des Konstruktors würde ich noch auf package private reduzieren.


Jetzt wo das Ding final ist? Gut, es gibt die Factory-Methoden in Either, aber wenn jemand lieber das `new Left` schreiben will, warum nicht?. Habe da aber keine starke Meinung dazu, vielleicht ist es so sicherer, falls wir noch etwas ändern. "Aufmachen" kann man es später ja immer noch...


> 
> Habe mir den Code aber noch nicht im Detail angesehen, das hole ich heute Abend nach.
> 
> Auf lazy-Initialize würde ich auch verzichten, weil die nonnull-Eigenschaft dabei nicht geprüft werden kann.



OK


> Bei mapLeft und mapRight würde ich casten. Allerdings nicht unbedingt aus Performancegründen, sondern um keine weiteren, überflüssigen Objekte zu erzeugen.


Habe ich auch keine starke Meinung dazu, und kann ja auch leicht wieder rückgängig gemancht werden, wenn es Probleme verursacht. Machen wir es so, falls Marco keinen Einspruch erhebt.

Ich denke, es ist OK, wenn ich meine Version mit den Anpassungen mal committe, weil es ja jetzt doch mehr um Details geht. Nebenbei bemerkt finde ich es gut, wie wir die Sache angehen, und dass der Code wirklich erst dann in Ruhe gelassen wird, wenn alle damit zufrieden sind. Schnell zusammenhacken kann jeder...

cmrudolph · 2. Juni 2014 um 09:08

Ich habe Objects#requireNonNull heute ein paar mal verwendet und festgestellt, dass der Rückgabewert gerade bei mehreren zu prüfenden Konstruktorargumenten äußerst praktisch ist. Ich denke, dass man sich an den Ausdruck gewöhnt: also mittlerweile auch „dafür“.

Sehe ich genauso. Ich sehe kein großes Risiko in einem öffentlichen Konstruktor, aber keine Notwendigkeit für ihn - daher die geringstmögliche Sichtbarkeit.

Ja, wenn noch was geändert werden soll, dann stört es ja nicht, einen commit mehr im log zu haben

*** Edit ***

Ich habe den Code mal reformatiert, Assert gelöscht und noch einen weiteren Punkt.

Vorerst habe ich im testHashCode() die assertFalse-Fälle gelöscht. MMn kann über eine fehlende Übereinstimmung von zwei Hashcodes keine Aussage gemacht werden. Und mit den verbliebenen Tests zu hashCode bin ich auch noch unglücklich, weil sie nicht auf den Kontrakt testen, sondern eher zufällig korrekt sind*. Man könnte höchstens testen, ob zwei Objekte den selben Hashcode haben, wenn der equals-Vergleich true ergibt. Alles andere ist überspezifiziert und daher mMn kein guter Test.

ok, in diesem Fall ist es natürlich kein Zufall, dass der Test grün ist, aber die Implikation, die dem hashCode zugrunde liegt „equals true [tex]\rightarrow[/tex] hashCode gleich“, geht nicht aus dem Test hervor.

Landei · 3. Juni 2014 um 04:44

Ich würde noch gerne eine Methode einfügen, die das Finden und Filtern erleichtern würde:

public abstract boolean test(Predicate<A> predA, Predicate<B> predB);

Könnte entsprechend auch in Tupeln implementiert werden.

cmrudolph · 4. Juni 2014 um 11:15

Die test-Methode könnte man zum Either meinetwegen hinzufügen. Bei Tupeln erscheint es mir aber nicht so offensichtlich, wie die einzelnen Prädikate dann miteinander verbunden werden sollen. Mit Und? Mit Oder? Oder beides mit testAnd und testOr? Ich würde dann die letzte Variante bevorzugen.

Landei · 4. Juni 2014 um 23:35

Zu genau dem Schluss bin ich nach etwas Nachdenken auch gekommen. Bei Tripeln und höher fehlen dann zwar trotzdem einige mögliche Kombinationen, aber in der Praxis sollten testAnd und testOr die meisten Fälle abdecken.

Landei · 6. Juni 2014 um 00:55

Mir ist aufgefallen, dass wir noch zwei Methoden analog zu Optional anbieten könnten:

public abstract <X extends Exception> A leftOrElseThrow(Supplier<? extends X> exceptionSupplier) throws X;

public abstract <X extends Exception> B rightOrElseThrow(Supplier<? extends X> exceptionSupplier) throws X;

cmrudolph · 6. Juni 2014 um 01:51

Ja, die xxxOrElseThrow-Methoden finde ich auch praktisch. Eine Verwendungsmöglichkeit hat man ja im List-Thread schon gesehen. Also: dafür.

Landei · 8. Oktober 2014 um 05:02

Habe mal die statischen Methoden joinLeft und joinRight zu Either hinzugefügt. Außerdem Left und Right package-private gemacht, nachdem ich gemerkt hatte, dass die exakten Rückgabewerte bei Either.leftOf/rightOf eher hinderlich sind.

Hier der Code in Either:

    public static <A, B> Either<A, B> leftOf(A a) { //gab früher Left<A,B> zurück
        return new Left<>(a);
    }

    public static <A, B> Either<A, B> rightOf(B b) { //gab früher Right<A,B> zurück
        return new Right<>(b);
    }

    public static <A, B> Either<A, B> joinLeft(Either<Either<A, B>, B> either) {
        return either.fold(ab -> ab, Either::rightOf);
    }

    public static <A, B> Either<A, B> joinRight(Either<A, Either<A, B>> either) {
        return either.fold(Either::leftOf, ab -> ab);
    }