HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Refactoring away from exceptions

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
refactoringexceptionsfromaway

Problem

What do you use for refactoring away from exceptions while programming functional in C#? I defined this class to hold function outcome:

public class Result 
{
    public static readonly Result OK = new Result();
    public static implicit operator Result(Exception error) => new Result(error);

    protected Result(Exception error = null)
    {
        Error = error;
    }

    public Exception Error { get; }
    public bool Succeeded => Error == null;
    public bool Failed => !Succeeded;

    public override string ToString() =>
        Succeeded ? "OK" : Error.Message;
}


And its subclass to hold return value where necessary:

public class Result : Result, IEnumerable
{
    public static implicit operator Result(Exception error) => new Result(error);
    public static implicit operator Result(T value) => new Result(value);
    public static implicit operator T(Result result)
    {
        if (result.Failed)
            throw result.Error;

        return result.Value;
    }

    protected Result(T value)
        : base(error: null)
    {
        Value = value;
    }

    protected Result(Exception error)
        : base(error)
    {
    }

    T Value { get; }

    IEnumerable Values =>
        Failed ? new T[0] : new[] { Value };

    public IEnumerator GetEnumerator() =>
        Values.GetEnumerator();

    IEnumerator IEnumerable.GetEnumerator() => 
        GetEnumerator();
}


It is enumerable, exposing 0..1 elements, so amount of if-statements can be reduced.

Usage:

var r1 = Div(
            Div(100, 10),
            Div(10, 2));

        var r2 = Div(
            Div(100, 10),
            Div(0, 0));

        Console.WriteLine(r1.Succeeded); // True
        Console.WriteLine(r1.Count()); // 1
        Console.WriteLine(r1); // 2

        Console.WriteLine(r2.Succeeded); // False
        Console.WriteLine(r2.Count()); // 0
        // Console.WriteLine(r2); // throws DivideByZeroException


Where:

```
static Result Div(Result a, Result

Solution

I don't know what to think of the entire thing yet but...

public bool Succeeded => Error == null;
public bool Failed => !Succeeded;


I find this should be only one property bool Success.

I've watched video series and generally I think it's a new and interesting idea that I need to try out. Your solution is a good start especially with the implicit operators but I'm missing a few features.

I'm not so fond of using the Result as a parameter. It has another implicit operator for converting it into a value so using actual types for parameters is sufficient and it's easier to work with pure parameters rather than dealing with Results everywhere (but maybe it's just an example). It's also safer because I'm going to throw an exception if a result is in an invalid state so implicit casting it for the parameter would fail when a method is called and not later when a parameter is used. I find this is better because it indicates a bug before the call and not inside the method that uses a result.

public static implicit operator T(Result result)
{
    if (result.Failed)
        throw result.Error;

    return result.Value;
}


I don't think this is a good implementation for this operator and I prefer getting the Value would throw like this

T Value => Success ? _value : throw new InvalidOperationException("Value isn't set because the result is in error state.");


informing me that I cannot use it because the Result is in an invalid state for it. This is the behaviour that I expect from this property and not some default value that I might use in case I forgot to check the Success flag.

After all I think I could also live with two flags for the result that I'd call

public bool Success => string.IsNullOrEmpty(Message) && Exception == null;


and

public bool Failure => !Success;


whether it should be Success or Succeeded or Failure or Failed is probably an entire new question for the English Language community but to me the former sounds better as it tells me whether the operation was a Success or a Failure and not if the Result succeeded. Well, tometo tomato ;-)

The current solution is prepared to work only with exceptions and to be used only in methods that can throw but if we wanted to make our API consistent then it should also be possible to use the Result for other methods that not necessarily throw but cannot continue because of some other reason so implementing factory methods like Ok and Fail and a custom message is not such a bad idea.

To me the perfect solution would go like this:

The non-generic Result can be created in a similar way as an exception this is with a message and an exception or with either one only. In case of a failure it returns either the exception's message or the custom one and could be used for methods that return void. For consistency reasons I turned the OK property into a Ok method.

public class Result
{
    protected Result(string message, Exception exception) { Message = message; Exception = exception; }
    protected Result(string message) : this(message, null) { }
    protected Result(Exception Exception) : this(null, Exception) { }

    public Exception Exception { get; }
    public string Message { get; }

    public bool Success => string.IsNullOrEmpty(Message) && Exception == null;
    public bool Failure => !Success;

    public static Result Ok() => new Result(nameof(Ok));

    public static Result Fail(string message, Exception exception) => new Result(message, exception);
    public static Result Fail(string message) => Fail(message, null);
    public static Result Fail(Exception exception) => Fail(null, exception);

    public override string ToString() => Success ? Message : (Exception?.Message ?? Message);

    public static implicit operator Result(Exception exception) => Fail(exception);
    public static implicit operator Result(string message) => Fail(message);
    public static implicit operator bool(Result result) => result.Success;
}


The generic implementation could provide a value:

```
public class Result : Result, IEnumerable
{
private readonly T _value;

protected Result(T value) : base(Exception: null) => _value = value;
protected Result(string message, Exception exception) : base(message, exception) { }
protected Result(string message) : base(message, null) { }
protected Result(Exception exception) : this(null, exception) { }

public T Value => Success ? _value : throw new InvalidOperationException("Value isn't set because the result is in error state.");

public static Result Ok(T value) => new Result(value);

public static new Result Fail(string message, Exception exception) => new Result(message, exception);
public static new Result Fail(string message) => Fail(message, null);
public static new Result Fail(Exception exception) => Fail(null, exception);

public IEnumerator GetEnumerator() => Enumerable.Repeat(Value, S

Code Snippets

public bool Succeeded => Error == null;
public bool Failed => !Succeeded;
public static implicit operator T(Result<T> result)
{
    if (result.Failed)
        throw result.Error;

    return result.Value;
}
T Value => Success ? _value : throw new InvalidOperationException("Value isn't set because the result is in error state.");
public bool Success => string.IsNullOrEmpty(Message) && Exception == null;
public bool Failure => !Success;

Context

StackExchange Code Review Q#157748, answer score: 3

Revisions (0)

No revisions yet.