add new method WithType to add types to a OneOf #171

Pxtl · 2024-02-17T05:36:34Z

In order that functions that return OneOf can call other functions that return
OneOf, the .WithType() method allows you to return existing OneOf structs but
with added types.

Note: Unfortunately because of the extended assembly design, OneOf types with
over 8 parameters cannot use .WithType.

public OneOf<string, double> ParseDouble(string input) {
    if (double.TryParse(input, out var result)) {
        return result;
    } else {
        return input;
    }
}

public OneOf<string, double, DateTime, int> ParseDoubleOrUTCDateOrInt(string input) {
    if (DateTime.TryParseExact(input, "yyyy-MM-dd", CultureInfo.InvariantCulture, DateTimeStyles.AdjustToUniversal | DateTimeStyles.AssumeUniversal, out var dateResult)) {
        return dateResult;
    }
    else if (int.TryParse(input, out var intResult)) {
        return intResult;
    } else {
        return ParseDouble(input).WithType<DateTime>().WithType<int>();
    }
}

resolves #150

- also added the README to the solution

jacob7395 · 2024-10-28T18:07:59Z

Generator/Program.cs

@@ -49,13 +50,13 @@ namespace OneOf
        readonly int _index;

        {IfStruct( // constructor
-        $@"OneOf(int index, {RangeJoined(", ", j => $"T{j} value{j} = default")})
+        $@"internal OneOf(int index, {RangeJoined(", ", j => $"T{j} value{j} = default")})


why add internal to these, it will break anyone's code who has inherited from these with these constructors

Because it's necessary. If you look in WithType, it uses the constructor of OneOfTN+1. This is also why WithType doesn't work with large size OneOfs - it can't access the constructors of the next size beyond extendedSizeLimit - 1 because those classes are located in a different assembly.

Okay, ye I see that for this constructor now, but the $@"protected internal OneOfBase(OneOf<{genericArg}> input)1 can stay as just protected?

That way it wont effect anyone who is inheriting.

jacob7395 · 2024-10-28T18:26:20Z

Generator/Program.cs

@@ -77,6 +78,12 @@ namespace OneOf

        public int Index => _index;

+        {((i < extendedSizeLimit - 1) ?
+        // can go up to the limit before extended because OneOfT8 cannot see OneOfT9
+        $@"public OneOf<{genericArg}, TNew> WithType<TNew>() =>


This feels like a good method but, I am not sure if this interface meets the use cases I have in mind. When this is used would it not makes sense to provide a value and set the index to that value.

I think the method should at least be provided.

I see what you mean - so in your case you're using this as a convenience object to construct a new object with a new WithType and its own value, and we're just going to throw out the old value-content.

That makes sense. I'd have to see how that works with the existing methods of OneOf. Is there some equivalent "use an existing object for a convenience-object for constructing a new one" thing to build off of?

Thinking it over, maybe I should be redoing this as .WithTN+1 so that it becomes something like

OneOf<T1, T2, T3> myOneOf = SomeFunc(); return myOneOf.WithT4<SomeClass>();

Then the case you describe could be done as a .FromTN+1; so OneOfT3 would have a .FromT4 for cases where you want to expand a T3 by one.

Or maybe just call that .FromNewType, which would make sense as a counterpart to .WithType, althought I don't know how much the .FromTN things get used.

I'm being silly, you can't call a static method from an instance so I think the only time you could ever use .FromTN+1 would be after literally defining var myOneOf = OneOf<T1, T2, T3>.FromT4<T4>(someValue). Why would you ever do that?

I wat thinking something like

public OneOf<T0, TNew> WithType<TNew>(TNew value) => value;

This seems to work;

{((i < extendedSizeLimit - 1) ? // can go up to the limit before extended because OneOfT8 cannot see OneOfT9 $@"public OneOf<{genericArg}, TNew> WithType<TNew>(TNew value) => value; ":"")}

Could still keep the WithType that doesn't take a value.

Sorry I was being way too verbose in my old replies, thinking out loud. My point: in this case you're essentially constructing a new OneOf and throwing out the old value. I mean, I see how it works but I don't get what it's for.

mcintyre321 · 2024-10-29T07:11:24Z

Hi, thanks for using OneOf enough enough to extend it. I think this feature would be better off in an external package, rather than in the main project, which I'm trying to keep lean. I can add a section to the readme with a link if you create your own project.

…

On Mon, 28 Oct 2024, 22:07 Martin Zarate, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In Generator/Program.cs <#171 (comment)>: > @@ -49,13 +50,13 @@ namespace OneOf readonly int _index; {IfStruct( // constructor - $@"OneOf(int index, {RangeJoined(", ", j => $"T{j} value{j} = default")}) + $@"internal OneOf(int index, {RangeJoined(", ", j => $"T{j} value{j} = default")}) Because it's necessary. If you look in WithType, it uses the constructor of OneOfTN+1. This is also why WithType doesn't work with large size OneOfs - it can't access the constructors of the next size beyond extendedSizeLimit - 1 because those classes are located in a different assembly. ------------------------------ In Generator/Program.cs <#171 (comment)>: > @@ -77,6 +78,12 @@ namespace OneOf public int Index => _index; + {((i < extendedSizeLimit - 1) ? + // can go up to the limit before extended because OneOfT8 cannot see OneOfT9 + $@"public OneOf<{genericArg}, TNew> WithType<TNew>() => I see what you mean - so in your case you're using this as a convenience object to construct a new object with a new WithType and its own value, and we're just going to throw out the old value-content. That makes sense. I'd have to see how that works with the existing methods of OneOf. Is there some equivalent "use an existing object for a convenience-object for constructing a new one" thing to build off of? ------------------------------ In Generator/Program.cs <#171 (comment)>: > @@ -77,6 +78,12 @@ namespace OneOf public int Index => _index; + {((i < extendedSizeLimit - 1) ? + // can go up to the limit before extended because OneOfT8 cannot see OneOfT9 + $@"public OneOf<{genericArg}, TNew> WithType<TNew>() => Thinking it over, maybe I should be redoing this as .WithTN+1 so that it becomes something like OneOf<T1, T2, T3> myOneOf = SomeFunc();return myOneOf.WithT4<SomeClass>(); Then the case you describe could be done as a .FromTN+1; so OneOfT3 would have a .FromT4 for cases where you want to expand a T3 by one. Or maybe just call that .FromNewType, which would make sense as a counterpart to .WithType, althought I don't know how much the .FromTN things get used. ------------------------------ In Generator/Program.cs <#171 (comment)>: > @@ -77,6 +78,12 @@ namespace OneOf public int Index => _index; + {((i < extendedSizeLimit - 1) ? + // can go up to the limit before extended because OneOfT8 cannot see OneOfT9 + $@"public OneOf<{genericArg}, TNew> WithType<TNew>() => I'm being silly, you can't call a static method from an instance so I think the only time you could ever use .FromTN+1 would be after literally defining var myOneOf = OneOf<T1, T2, T3>.FromT4<T4>(someValue). Why would you ever do that? — Reply to this email directly, view it on GitHub <#171 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACDJ6UFWXYIUMBI4LYZU5LZ52YSNAVCNFSM6AAAAABQYAN6COVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIMBQGMYDAMZXGQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

jacob7395 · 2024-10-29T10:53:12Z

Hi, thanks for using OneOf enough enough to extend it. I think this feature would be better off in an external package, rather than in the main project, which I'm trying to keep lean. I can add a section to the readme with a link if you create your own project.

The issue is functionality like this requires the use a private (made internal with this PR), what is your reason behind trying to keep it lean. Functionality like this PR are more syntactic sugar.

Pxtl · 2024-10-29T13:58:28Z

@mcintyre321

Hi, thanks for using OneOf enough enough to extend it. I think this feature would be better off in an external package, rather than in the main project, which I'm trying to keep lean. I can add a section to the readme with a link if you create your own project.

I mean it's possible but the API that makes WithType pretty slick to implement and also pretty performant is private. Without a public constructor OneOf<T0...TN>(int index, T0 value0, ... TN valueN) it becomes much less practical. Are there other forms of this you might be interested in? I might play with the reverse approach... allowing "smaller" one-ofs have an implicit cast to "larger" one-ofs as a way to implement the same feature.

Pxtl · 2024-10-29T17:13:07Z

@mcintyre321

I've added an alternate typecast-based approach here:

#185

add new method WithType to add types to a OneOf

384da24

- also added the README to the solution

Pxtl force-pushed the withType branch from e132e80 to 384da24 Compare February 17, 2024 05:41

Pxtl mentioned this pull request Oct 23, 2024

Return "partial" OneOf types #181

Open

jacob7395 reviewed Oct 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add new method WithType to add types to a OneOf #171

add new method WithType to add types to a OneOf #171

Pxtl commented Feb 17, 2024 •

edited

Loading

jacob7395 Oct 28, 2024

Pxtl Oct 28, 2024

jacob7395 Oct 29, 2024 •

edited

Loading

jacob7395 Oct 28, 2024

Pxtl Oct 28, 2024

Pxtl Oct 28, 2024

Pxtl Oct 28, 2024

jacob7395 Oct 29, 2024

Pxtl Oct 29, 2024

mcintyre321 commented Oct 29, 2024 via email

jacob7395 commented Oct 29, 2024 •

edited

Loading

Pxtl commented Oct 29, 2024 •

edited

Loading

Pxtl commented Oct 29, 2024

add new method WithType to add types to a OneOf #171

Are you sure you want to change the base?

add new method WithType to add types to a OneOf #171

Conversation

Pxtl commented Feb 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacob7395 Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcintyre321 commented Oct 29, 2024 via email

jacob7395 commented Oct 29, 2024 • edited Loading

Pxtl commented Oct 29, 2024 • edited Loading

Pxtl commented Oct 29, 2024

Pxtl commented Feb 17, 2024 •

edited

Loading

jacob7395 Oct 29, 2024 •

edited

Loading

jacob7395 commented Oct 29, 2024 •

edited

Loading

Pxtl commented Oct 29, 2024 •

edited

Loading