2

Looking for the best way to use OneOf multiple times, returning early depending on the result, specifically if you can get an Error along the way.

For example, consider a builder where you need to build individual parts and then combine them. Each part builder returns a OneOf<ThisPart, Error>, so for every step along the way, we might need to return early with that error.

// Assume these have real implementations and can return errors:
OneOf<SteeringWheel, Error> BuildSteeringWheel() => new SteeringWheel();
OneOf<Engine, Error> BuildEngine() => new Engine();
OneOf<Stereo, Error> BuildStereo() => new Stereo();

public record class Car(SteeringWheel SteeringWheel, Engine Engine, Stereo Stereo);
public record class SteeringWheel();
public record class Engine();
public record class Stereo();

To build a car, I need each of the three components, and return early if any component builder gives me an error. I can think of a few ways, but nothing feels right:

// This one is ugly and would really get nasty the more complexity you add
OneOf<Car, Error> BuildCar_Nested()
{
    return BuildSteeringWheel().Match(
        steeringWheel => BuildEngine().Match(
            engine => BuildStereo().Match<OneOf<Car, Error>>(
                stereo => new Car(steeringWheel, engine, stereo),
                stereoError => stereoError
            ),
            engineError => engineError
        ),
        steeringWheelError => steeringWheelError
    );
}

// The AsT1 property feels like a cheat, and pretty much provides the same use
// as if I would have returned a (T, Error) tuple, which kills the point
// of using OneOf. Same would be true if I used the TryPickT0 methods
OneOf<Car, Error> BuildCar_AsT()
{
    OneOf<SteeringWheel, Error> steeringWheel = BuildSteeringWheel();
    if (steeringWheel.IsT1)
    {
        return steeringWheel.AsT1;
    }

    OneOf<Engine, Error> engine = BuildEngine();
    if (engine.IsT1)
    {
        return engine.AsT1;
    }

    OneOf<Stereo, Error> stereo = BuildStereo();
    if (stereo.IsT1)
    {
        return stereo.AsT1;
    }

    return new Car(steeringWheel.AsT0, engine.AsT0, stereo.AsT0);
}

// This forces me to deal with all result types by using Switch, but it
// also makes me check the error each time, and do redundant work by null-checking
// something that is definitely not null, or some other workaround. Again this
// pattern is pretty much no different from returning a Tuple of (T, Error).
// Also trying not to use the ! operator if possible.
OneOf<Car, Error> BuildCar_Switches()
{
    Error? error = null;

    SteeringWheel? steeringWheel = null; 
    BuildSteeringWheel().Switch(
        sw => steeringWheel = sw,
        err => error = err
    );

    if (error is not null) { return error.Value; }

    // This avoids compiler warnings
    // Or I could have done SteeringWheel steeringWheel = null!; above
    if (steeringWheel is null) { return new Error(); }

    Engine? engine = null;
    BuildEngine().Switch(
        e => engine = e,
        err => error = err
    );
    
    if (error is not null) { return error.Value; }
    if (engine is null) { return new Error(); }

    Stereo? stereo = null;
    BuildStereo().Switch(
        s => stereo = s,
        err => error = err
    );

    if (error is not null) { return error.Value; }
    if (stereo is null) { return new Error(); }

    return new Car(steeringWheel, engine, stereo);
}

Is there a clean and proper way of doing this? The Switch approach seems to be the least incorrect and the approach I'll take if I have to, but I'm still looking for a better way.

Improve this question

4 Answers 4

Reset to default
6

Anytime I see something like this that goes against long established conventions, I look for these things:

  1. Does the new thing make it easier to test the creation of a new Car?

  2. Does the new thing make implementing the logic for creating a Car easier?

  3. Does the new thing make it significantly easier for callers who want to construct a car and handle the errors?

  4. Does the new thing solve a technical problem with creating cars much more elegantly than other more conventional methods?

You don't need to answer "yes" to all of those questions. As with most things in software engineering, it's about deciding between trade-offs. The net effect should be code that is significantly easier to test, or significantly easier to understand — ideally both. This would justify throwing conventions out the window. The overhead to understand a new style is offset by reduced complexity overall.

In my personal opinion, this might not be the best use case for the functional style that OneOf enables. You can get it to work, but is it easier to reason about? Based on your own assessment of the code in your question, I don't think you believe it is.

To be honest, I've seen plenty of builders in my career, and this is nigh impossible to understand. The functional style seems forced in a situation where exceptions make code a lot cleaner because execution stops at the point of failure. The version using OneOf still cannot complete object construction if an error occurs, but execution continues anyways. Admittedly, this might be an artifact of how you are using OneOf rather than a design flaw with the library or pattern.

Another aspect that seems off is that every error returned is the same type. This eliminates the elegance that OneOf.Match(...) provides because you no longer have return values specific to certain errors. Callers are given a generic "something went wrong" object that is not semantically different than catch (Exception error). Even if you split errors into their own types, what can the caller do about an InvalidEngineException?

When in doubt, first write some example code from the perspective of the caller. If the calling code becomes harder to test or reason about, the burden of justifying use of a new style falls to the implementing code. That code should be a lot easier to test or reason about. If not, then you might not be using the right tool for the job. It's not a bad tool, just maybe not the right problem.

Improve this answer
2

This reads like a better use case of a tryXXXXX() pattern, it would let you break out early on any failures to handle errors better.

bool tryBuildSteeringWheel(input, out OneOf<SteeringWheel, Error> results) 
bool tryBuildEngine(input, out OneOf<Engine, Error> results)
bool tryBuildStereo(input, out OneOf<Stereo, Error> results)

usage might look something like this:

if (tryBuildSteeringWheel(input, out OneOf<SteeringWheel, Error> wheelResults) 
   && tryBuildEngine(input, out OneOf<Engine, Error> engineResults) 
   && tryBuildStereo(input, out OneOf<Stereo, Error> stereoResults)){
   //create your car  
}
else {
   //handle error results
}

This will short circuit and return on the first failure, you can handle the error appropriately there.

Improve this answer
1
  • 1
    That’s an interesting idea - I don’t think it would work without resorting to the T1/T2 pattern or a bunch of empty or redundant switch cases, but it does give another way of looking at it. Commented May 11, 2024 at 0:10
1

This OneOf library is implementing a Functor1, but what you'd really like is for it to be an Applicative too. However, it's relatively easy to write an extension method for that yourself.

Unfortunately you'll have to repeat this for each arity of function you want to use, but instead of an ever-growing repetition of implementations you have some fixed number.

public static OneOf<Error, TResult> Apply<T0, T1, T2, TResult>(
    this Func<T0, T1, T2, TResult> Func, 
    OneOf<Error, T0> V0, 
    OneOf<Error, T1> V1, 
    OneOf<Error, T2> V2)
// Any of your implementations of BuildCar here
  1. Actually it's multiple different functors, as each generic type has a corresponding Map method.
Improve this answer
1
  • Can you maybe explain it a bit more? I feel I understood your answer, but if the link ever goes dead, nobody else will. Commented May 25 at 8:13
-2

How about this? (Pretend that TryPickT1 was named IsError and it'll make more sense)

OneOf<Car, Error> BuildCar_TryPickT1()
{
    if (BuildSteeringWheel().TryPickT1(out var error, out var steeringWheel) ||
        BuildEngine().TryPickT1(out error, out var engine) ||
        BuildStereo().TryPickT1(out error, out var stereo))
    {
        return error;
    }

    return new Car(steeringWheel, engine, stereo);
}
Improve this answer

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.