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

Possibility to implement TryPick<Tx> #151

Open
aliriocastro opened this issue Jul 5, 2023 · 10 comments
Open

Possibility to implement TryPick<Tx> #151

aliriocastro opened this issue Jul 5, 2023 · 10 comments

Comments

@aliriocastro
Copy link

Hello Team, I'm just using the library and I feel fascinated. However, I would like to propose the following and check if it is possible.

I just built the code to explain myself:

public bool TryPick<T0>(out T0 value, out T1 remainder) {
            value = IsT0 ? AsT0 : default;
               remainder = _index switch
            {
                0 => default,
                1 => AsT1,
                _ => throw new InvalidOperationException()
            };
            return this.IsT0;
 }

The point is, instead of forcing to T[index], we could use the type.

I hope I could explain myself.

@cremor
Copy link

cremor commented Jul 6, 2023

Your code sample doesn't work as provided. It produces an error ("CS0029 Cannot implicitly convert type 'T0' to 'T0'") and a warning ("CS0693 Type parameter 'T0' has the same name as the type parameter from outer type 'OneOf<T0, T1>'").

If you rename the type parameter (but not the type of the first parameter) then it would compile. But then it doesn't make any sense since you would always try to get T0 instead of the type provided on the method call.

@mcintyre321
Copy link
Owner

I held off doing it this way when I wrote OneOf in case people ever found that the types they were using were covariant.

In my old age I'm softening on this and thinking that having generic Is TryPick etc would be good, but I wouldn't want to remove the old methods, and I wouldn't want to balloon the assembly size with new methods. What to do?

@cremor
Copy link

cremor commented Jul 8, 2023

I wouldn't want to balloon the assembly size with new methods

Would those methods make such a big difference? Like going from 100 kB (what we have now) to 1 MB or something like that?
And even if the difference was that big: Would it really matter? The only place where I can see assembly size being relevant is Blazor (WebAssembly). But for that we have assembly trimming.

I have more concerns about the correctness of such a method.

  • How would you implement the type check? (Type.Equals() or is?)
    • If you use is, how would you handle the case when the given type matches multiple types (like one being a direct match and one being a subclass)?
  • How would you handle the case if you have multiples of the same type (like OneOf<TypeA, TypeB, TypeA>)? I think this shouldn't be done, but someone could have code like that.

@wdolek
Copy link

wdolek commented Aug 7, 2023

Additionally @aliriocastro, how would TryPick look for bigger type arity, let's say OneOf<T0, T1, T2>? Would you return two reminders, or this should be implemented just for arity = 2? (I'm struggling to see benefits of having this for type with more than two type args)

@jamesbascle
Copy link

@wdolek A pattern I've been using is something like

if(oneOf.TryPickT0(out Success _, out var errors)

That way your happy path happens in a block, then error handling happens in a normal switch call. If you repeat this pattern, it also doesn't tie consumers of OneOf's to be tied to the author's generics order. That was actually one of the original reasons I was interested in the approach we took with DiscU some years ago. I think the TryPick pattern is pretty good and offers a reasonable balance between rigour and flexibility.

As I mentioned in another issue though, it does introduce the possibility for subtle bugs if you use var, then add another type to the OneOf not at the end of the current list of generic arguments, your logic changes without warning because there is no enforced semantic binding between the generic type and consuming code due to the TryPickTn methods being position-based, rather than generic-based.

@wdolek
Copy link

wdolek commented Sep 14, 2023

@jamesbascle if I understand it correctly (I'm struggling to find anything related to this in DiscU), proposal for OneOf<T0, T1, T2> is either:

Implement every combination of Tn (this won't scale and prevent use of var):

  • bool TryPickT0(out T0 v, out T1 r)
  • bool TryPickT0(out T0 v, out T2 r)
  • bool TryPickT1(...) ...

Implement generics (but I don't see how to constraint TR to any of Tn):

  • bool TryPickT0<TR>(out T0 v, out TR r)

Wrap remainder in another OneOf<T1, T2> but it again needs generating all combinations:

  • bool TryPickT0(out T0 v, out OneOf<T1, T2> r)
  • bool TryPickT1(out T1 v, out OneOf<T0, T2> r)
  • ...

Could you extend your example? When I tried to implement my own DU, I ended up with similar pattern as in OneOf, just without having Tn suffix in method name - still working with one type you are trying to pick. (You can find project in my profile)

@jamesbascle
Copy link

jamesbascle commented Sep 15, 2023

@wdolek

if you have this OneOf

[GenerateOneOf]
public partial class Example : OneOfBase<Success, Error1, Error2, Error3>{}

public record Error1();
public record Error2();
public record Error3();

Then you can write some code that looks like this

 Example x = new Error2();
 if (x.TryPickT0(out Success _, out var err1))
 {
     //...
 }
 else if (err1.TryPickT2(out Error3 _, out var err2))
 {
     //...
 }
 else if (err2.TryPickT1(out Error2 _, out var err3))
 {
     //...
 }
 else
 {
 }

When I mentioned DiscU, one of the things I was looking to get away from was the imposition on the consumer of a DU, by the author of it. Specifically that imposition is that if you want to use the Match method, you are forced to write your logic in the same order in which the generic parameters to the OneOf are provided. The TryPickTn methods, in a similar manner to the DiscU matcher, allows you to write your logic in the way you would like, without being bound to the order of the generic parameters.

In practice, I think something like the following example controller method stub is a pretty good pattern as well.

        if (x.TryPickT0(out Success _, out var err))
        {
            //...
        }

        return err.Match(
            (Error1 e) => BadRequest(),
            (Error2 e) => NotFound(),
            (Error3 e) => Conflict());

The TryPickTn methods provide a lot of flexibility in implementation. Like I said though, one of the problems I see is there being no enforced semantic binding between the n and the type, so in certain scenarios using var this could prove problematic. The solution is to prohibit the use of var in the first out parameter.

@wdolek
Copy link

wdolek commented Sep 15, 2023

Re first example, it could just be:

if (x.TryPickT0(out Success s))
{
}
else if (x.TryPickT1(out Error1 e1))
{
}
// ...

... right? Just keep trying to pick value from x and not from remainder. Also doing it this way will be less confusing, because T0 will be always T0 and so on - in your example, I feel it would be quite error prone turning T3 to T2 in second condition 😕


Second example suggests err is OneOf<TEerror1, TError2, TError3> - but here you mix imperative escape hatch (TryPickT0 inside if) with functional matching. Why not match success right away alongside errors? It could easily be (static) method group handling it, I can't see benefit of doing it using your example.


Nevertheless, I would dare to say that arity above two is niche use case. For example, in my team we have introduced base Error class and specific error types extending it. That way, you can use OneOf<TSuccess, Error> and use pattern matching to distinguish which exact error happened for T1.

@jamesbascle
Copy link

jamesbascle commented Sep 15, 2023

@wdolek Like I said, It feels to me like an imposition upon the user to force them to write their logic in a particular order, in order to get the benefits of exhaustive type checking. That was the essentially goal of DiscU - allow users to write their logic in any manner they wish, but still provide the exhaustive type checking. It does that in a similar manner to the TryPickTn methods, by repeatedly reducing the arity, and running the code if applicable, but using a state-machine based fluent interface pattern instead of the out parameter pattern.

The pattern you've shown w/ the repeated if blocks doesn't do exhaustive type checking, and is functionally equivalent to just calling .IsTn or .AsTn with some if blocks.

I've decided that I'd rather use a more bog standard library such as OneOf that integrates well with various other parts of the ecosystem, like F#, and that people are more likely to have used elsewhere. It's also just a fact that OneOf is optimized for speed in a way an approach like DiscU would have a hard time matching. With some minor guardrails, OneOf is still a great experience, and the rest of my dev team gave it the thumbs up too.

@andrew-bauer-calibrated
Copy link

I was going to ask for the same function signature with a different output. I am after TryPick to give me the output if it is of type T, regardless of the position within the OneOf.

This is extremely useful for scenarios like custom state records, where each of the types in the OneOf would be different. For instance, doing a TryPick on a OneOf<Off, Idle, Active>, would give back the Active record if it is set.

To handle where a type is used more than once, you could return a new OneOf encapsulating all of them. For instance, doing a TryPick on a OnefOf<string, int, string>, would return OneOf<string, string>.

Not sure if this is possible but would be extremely useful.

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

No branches or pull requests

6 participants