azjezz / psl

📚 PHP Standard Library - a modern, consistent, centralized, well-typed, non-blocking set of APIs for PHP programmers
MIT License
1.22k stars 71 forks source link

Result is too restrictive, introduce Either type? #355

Open someniatko opened 2 years ago

someniatko commented 2 years ago

I want to type a function/method representing action which may fail, but not from pure technical perspective, rather from the business logic one. For instance, "register new user" action may succeed, or may fail for reasons like "username is already taken" or "password is too short" etc. I don't want to split this action into two steps, a validation method and then the actual action method, because I don't like creating an intermediate state which is invalid (however others may want to do it in two steps — it's just style preference, I made this choice).

This lib has a Result type which looks like it's exactly what I need, but it's unfortunately not: its Failure branch is limited only to exceptions (more correctly, throwables). However this is not how I view the business logic: for me exceptions are, well, exceptional situations: e.g. "database connection failed" or "file could not be read", "remote API returned HTTP 500" etc. Purely technical ones. Or could be business-logic related, but unexpected, e.g. some entity's invariant was violated due to some logic error in the code, or maybe someone edited its state directly in the database etc. This is the other thing though, i don't want any exception-related fluff here: traces, codes etc. Such action "failures" are no more than just normal operation of my system.

Describe the solution you'd like Ideally I'd like to extend Result failure type to accept not only Exceptions. This is not possible due to possible backwards-compatibility break. Instead, a new type can be introduced: Either, with Left and Right variants as inspired by Haskell.

Describe alternatives you've considered I have my own library for this, https://github.com/someniatko/php-result-type, which was inspired by this one: https://github.com/GrahamCampbell/Result-Type, but it has more robust Psalm typing.

azjezz commented 2 years ago

I'm okay with adding this feature to PSL, however, i don't think i have the time to implement this myself, PRs are welcome :)

veewee commented 2 years ago

You might find some inspiration in this version as well https://github.com/marcosh/lamphpda/blob/master/src/Either.php

someniatko commented 2 years ago

@veewee Thank you! This lib truly is a gem, however it has built some sort of "functional infrastructure" with Functor, Monad typeclasses etc. I am not sure we can and should recreate something like that in PSL.

azjezz commented 2 years ago

personally, I imagine the Either API in PSL to look like this:

final class Either<Tr, Tl> {
   // static factories 
  public static function right<T>(T $val): Either<T, _>;
  public static function left<T>(T $val): Either<_, T>;

  // Throw MissingValueException if not right 
  public function getRight(): Tr;
  // Same
  public function getLeft(): Tl;

  public function isRight(): bool;
  public function isLeft(): bool;

  // could be useful
  public function wrapRight(): Result<Tr, MissingValueException>;
  public function wrapLeft(): Result<Tl, MissingValueException>;
}
someniatko commented 2 years ago

IMO to complete the API it would be great to have at least those:

/** @var callable(Tr):TrNew $map */
public function mapRight(callable $map): Either<Tr|TrNew, Tl>;

/** @var callable(Tl):TlNew $map */
public function mapLeft(callable $map): Either<Tr, Tl|TlNew>;

public function getEither(): Tr|Tl;

BTW, by having getEither() IDK if we need separate getRight() and getLeft(), which may throw exception.

someniatko commented 2 years ago
/** @var callable(Tr):Either<TrNew, TlNew> $map */
public function flatMapRight(callable $map): Either<Tr|TrNew, Tl|TlNew>;

/** @var callable(Tl):Either<TrNew, TlNew> $map */
public function flatMapLeft(callable $map): Either<Tr|TrNew, Tl|TlNew>;

may also be desirable

veewee commented 2 years ago

Maybe also a proceed($onLeft, $onRight): T similar to the result class?

azjezz commented 2 years ago

BTW, by having getEither() IDK if we need separate getRight() and getLeft(), which may throw exception.

I think we should, getEither would return Tr|Tl, when isRight returns true, i know that getEither would return Tr, but psalm/phpstan would assume it's Tr|Tl, having getRight() allows me to use it, knowing it won't throw since isRight() returned true.

someniatko commented 2 years ago

The whole point of Either is that you don't know which of the two outcomes will you get beforehand. You can still process its individual "branches" using mapLeft() / mapRight() or proceed() as @veewee suggests

azjezz commented 2 years ago

I'm in favor of proceed($fr, $fl) to keep consistency, however, getEither IMO, shouldn't not be a thing, Either<Tr, Tl> represents either one of those two types in question ( Tr and Tl ), if you want to work on both, you would use proceed<Tr2, Tl2>((Closure(Tr): Tr2) $fr, (Closure(Tl): Tl2) $fl): Either<Tr2, Tl2> is what you want to use, which would return another Either instance, if you want to map one of the branches, you would use mapRight<Tr2>((Closure(Tr): Tr2) $fr): Either<Tr2, Tl> ( same for mapLeft<Tl2>((Closure(Tl): Tl2) $fl): Either<Tr, Tl2> ), if you want to unwrap the value, in most cases, you would want to know which value you are getting, so what i would suggest instead of getRight/getLeft/wrapRight/wrapLeft is:

public function unwrap(): Tr|Tl;

public function unwrapRight(): Tr; // throws otherwise
public function unwrapLeft(): Tr; // throws otherwise

public function unwrapRightOr<T>(T $value): Tr|T; // returns `$value` otherwise
public function unwrapLeftOr<T>(T $value): Tr|T; // returns `$value` otherwise
azjezz commented 2 years ago

rust either type has many other methods that we could also implement ( see: https://docs.rs/either/latest/either/enum.Either.html#method.left_or_else )

public function getRight(): Tr; // throws otherwise
public function getLeft(): Tr; // throws otherwise

public function getRightOr<T>(T $value): Tr|T; // returns `$value` otherwise
public function getLeftOr<T>(T $value): Tl|T; // returns `$value` otherwise

public function getRightOrThen<T>((Closure(): T) $f): Tr|T; // returns `$f` results otherwise
public function getLeftOrThen<T>((Closure(): T) $f): Tl|T; // returns `$f` results otherwise

public function getRightOrElse((Closure(Tl): Tr) $f): Tr; // returns `$f` results otherwise
public function getLeftOrElse((Closure(Tr): Tl) $f): Tl; // returns `$f` results otherwise

public function map<Tm>((Closure(Tr|Tl): Tm) $f): Either<Tm, Tm>;
public function mapRight<Tr2>((Closure(Tr): Tr2) $f): Either<Tr2, Tl>;
public function mapLeft<Tl2>((Closure(Tl): Tl2) $f): Either<Tr, Tl2>;

( note: all methods prefixed with unwrap in rust either type return Option<T> which we don't have here, returning ?T is not an option as null could be a valid T value. )

azjezz commented 2 years ago

example ( stupid, and you shouldn't use Either in this case, but it gives you the idea 😛 ) :

function get_organization_owner(Either<Organization, Project> $either): Owner
{
  return $either->getLeftOrElse(
    fn(Project $project): Organization => $project->getOrganization()
  )->getOwner();
}

$owner = get_organization_owner(Either::left($organization));
$owner = get_organization_owner(Either::right($project));
azjezz commented 2 years ago

@someniatko wdyt?

someniatko commented 2 years ago

@azjezz it totally makes sense to me with those three unwrap functions you've suggested. Basically, unwrapRight(), unwrapLeft() and unwrap() are just renamed getLeft(), getRight() and getEither() from the previous comments. And in Haskell there is either() function which is basically in our terms proceed() + unwrap().

azjezz commented 2 years ago

what do you think about the other ones ( *Or, *OrElse, *OrThen )?

someniatko commented 2 years ago

Hmm, I looked a bit more closely, the proceed() method of the Result actually also unwraps it as well. So if we want to do it consistently, proceed() must also unwrap. I see it as a convenience method over mapLeft() + mapRight() + unwrap(), however maybe there is some deeper functional sense in it, IDK.

someniatko commented 2 years ago

As to unwrapLeft() / unwrapRight() functions which in Rust return an Option, why then not implementing Option first?

someniatko commented 2 years ago

If we implement an Option and return it for the unwrapX() methods, then we don't actually have to use those or / orThen methods on Either itself, we could just have them on the Option:

$either
    ->unwrapRight()
    ->orThen(fn() => computeSomething());

I am not sure about orElse though, thinking. It's basically some sugar for proceed($fr, $fl) without providing one of the callables, right?

someniatko commented 2 years ago

By the way, if proceed will also extract the value as either() in Rust and Haskell, then, I guess, separate unwrap() (getEither() as I suggested earlier) method is indeed not needed.

Just summarizing the discussion, the full API for could look like this:

// static factories 
public static function right<T>(T $val): Either<T, _>;
public static function left<T>(T $val): Either<_, T>;

public function isRight(): bool;
public function isLeft(): bool;

public function map<Tm>((Closure(Tr|Tl): Tm) $f): Either<Tm, Tm>;
public function mapRight<Tr2>((Closure(Tr): Tr2) $f): Either<Tr2, Tl>;
public function mapLeft<Tl2>((Closure(Tl): Tl2) $f): Either<Tr, Tl2>;

public function flatMap<Tmr,Tml>((Closure(Tr|Tl): Either<Tmr,Tml>) $f): Either<Tmr, Tml>;
public function flatMapRight<Tmr,Tml>((Closure(Tr): Either<Tmr,Tml>) $f): Either<Tr|Tmr, Tl|Tml>;
public function flatMapLeft<Tmr,Tml>((Closure(Tl): Either<Tmr,Tml>) $f): Either<Tr|Tmr, Tl|Tml>;

public function proceed<Tm>((Closure(Tr): Tm) $fr, (Closure(Tl): Tm) $fl): Tm;

public function unwrapRight(): Option<Tr>;
public function unwrapLeft(): Option<Tl>;

// could be useful
public function wrapRight(): Result<Tr, MissingValueException>;
public function wrapLeft(): Result<Tl, MissingValueException>;
azjezz commented 2 years ago

@someniatko Option implemented in #356, would appreciate your review :)

simPod commented 6 months ago

Is someone planning to work on this and has the final interface been decided?

veewee commented 6 months ago

I dont think anyone is planning on working on this at the moment, so feel free to pick it up. About the interface, you could start with the one described in https://github.com/azjezz/psl/issues/355#issuecomment-1172602495 maybe? It can always be extended.