Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested calls to multiple methods returning OneOf<T1, T2, ...> #150

Open
theplankmeister opened this issue May 31, 2023 · 3 comments · May be fixed by #171
Open

Nested calls to multiple methods returning OneOf<T1, T2, ...> #150

theplankmeister opened this issue May 31, 2023 · 3 comments · May be fixed by #171

Comments

@theplankmeister
Copy link

I'm really enjoying the concept of discriminated unions, but I'm thinking perhaps my understanding of OneOf's implementation is somewhat lacking. In my codebase, I have implemented OneOf in several packages that are common between projects. Some consuming classes have dependencies on more than one of these common packages, and each one returns relevant types in each their public methods. In multiple places in my codebase now, I'm having to implement conditional checking using IsTn because it's no longer semantically possible to use Match or Switch. I've prepared a (very much simplified) example:

    public class OneOfExample
    {
        public OneOf<NotFound, Error, Expired, Success> PerformSomeActionOrOther(string id)
        {
            OneOf<NotFound, B2CUser> getUserResult = GetUser(id);
            if (getUserResult.IsT0)
            {
                return getUserResult.AsT0;
            }

            B2CUser user = getUserResult.AsT1;
            user.AccountEnabled = true;
            user.RequiresMigration = false;

            OneOf<Error, Success> updateResult = UpdateUser(user);
            if (updateResult.IsT0)
            {
                return updateResult.AsT0;
            }

            return updateResult.AsT1;
        }

        private OneOf<NotFound, B2CUser> GetUser(string id)
        {
            B2CUser? user = ExternalRepository.GetUser(id);
            if (null == user)
            {
                return new NotFound();
            }

            return user;
        }

        private OneOf<Error, Success> UpdateUser(B2CUser user)
        {
            bool updated = ExternalRepository.UpdateUser(user);

            return updated ? new Success() : new Error();
        }
    }

Here we can see that the return values from the three methods are unique. Even if it made sense to do so, return getUserResult won't compile because OneOf<NotFound, B2CUser> does not match OneOf<NotFound, Error, Expired, Success>. But because I need to return early, it means I have to use TryPickn or IsTn, as shown in this example.

In my real codebase, due to the nature of the dependencies and their possible return types, this has led to an explosion of conditional returns that visually pollute the codebase and make it less readable.

So, my question is... Is there something I am missing, some concept or feature that would simplify my multitude of conditions? Or is this just a price of using discriminated unions in a statically-typed language?

@BrunoJuchli
Copy link

BrunoJuchli commented Jul 5, 2023

I personally combine OneOf with LanguageExt and for scenarios as these I combine Either with oneof.
Code then looks like below.

It's not perfect since you still need to manually specify that you want to convert an Error into an OneOf<NotFound, Error, Expired>.
And when you have stuff like converting a OneOf<NotFound, Error> into an OneOf<NotFound, Error, Expired>, you still have to write code for that yourself.
Instead of using OneOf<...> I personally like to introduce custom types like PerformSomeActionOrOtherError : OneOf<.....>. These can also be a good place to put the conversion logic into.

public class OneOfExample
{
    public Either<OneOf<NotFound, Error, Expired>, Success> PerformSomeActionOrOther(string id)
    {
        return GetUser(id)
          .MapLeft(notFound => 
              new OneOf<NotFound, Error, Expired>(notFound))
          .Bind(user =>
          {
              user.AccountEnabled = true;
              user.RequiresMigration = false;
              return UpdateUser(user)
                .MapLeft(error => new OneOf<NotFound, Error, Expired>(error));
          });
    }

    private Either<NotFound, B2CUser> GetUser(string id)
    {
        B2CUser? user = ExternalRepository.GetUser(id);
        if (null == user)
        {
            return new NotFound();
        }

        return user;
    }

    private Either<Error, Success> UpdateUser(B2CUser user)
    {
        bool updated = ExternalRepository.UpdateUser(user);

        return updated ? new Success() : new Error();
    }
}

@JoepGeevers
Copy link

JoepGeevers commented Aug 24, 2023

By making the generic type of Match explicit, I got this to work:

public OneOf<Color, NotFound, Expired> GetColor()
	=> ReadColor()
		.Match<OneOf<Color, NotFound, Expired>>(
			color =>
			{
				if (false)
				{
					return new Expired();
				}

				return color;
			},
			notfound => notfound);

private OneOf<Color, NotFound> ReadColor() => new NotFound();

public class NotFound { }
public class Expired { }

So even though the return type of ReadColor does not have Expired, the compiler can implicitly convert each of the two values to the return type of GetColor

@Pxtl
Copy link

Pxtl commented Feb 17, 2024

I've been playing with OneOf and it would be nice to have a command to expand a OneOf for cases like this.

Like

OneOf<T1, T2, T3,... TNplus1> OneOf<T1, T2, T3,... TN>.WithType<TNplus1>() {
  return new OneOf<T1, T2, T3,... TNplus1>(/* some kind of magic that gets the values from OneOf<T1, T2, T3,... TN> */);
}

Then it's easy to passthrough an existing OneOf or bolt on additional possibilities as needed.

Obviously it's quite brittle since you wouldn't be free to fully rearrange the OneOf. That way madness lies. But the trivial case of wrapping a method and adding one or more possible output types becomes easy as you do MyMethod.WithType<AnotherPossibleType>().

@Pxtl Pxtl linked a pull request Feb 17, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants