Result class which wraps another object

Question

I have a result class which wraps another object if successful or an error if failure

public class Result
{
    private Result() { }

    public bool Success { get; private set; }
    public string Error { get; private set; }
    public MyClass MyClass { get; private set; }

    public static Result CreateSuccess(MyClass myClass)
    {
        return new Result { Success = true, MyClass = myClass };
    }
    public static Result CreateFailure(string error)
    {
        return new Result { Error = error };
    }

    // ... see below
}

1. Is this a reasonable way to go about this?

I also am considering putting an implicit conversion to bool

public static implicit operator bool(Result result)
{
    return result.Success;
}

So that I can do

var result = GetResult();

if (!result)
{
    Print(result.Error);
}

2. Is this a silly idea?

Answer 1

Your result wrapper sounds like a good idea. I would modify it by allowing a collection of error strings, in case of multiple errors, warnings and the like, or perhaps consider holding an inner exception type (depending on how you raise these errors).

I am, on the other hand, wondering why you've hidden the constructor and opted for two static factory methods. As far as I can tell they offer nothing over simply offering two constructors instead, and they make for a less intuitive creation process for other developers (who will assume a constructor before trying other options):

public class Result
{
    public bool Success { get; private set; }
    public string Error { get; private set; }
    public MyClass MyClass { get; private set; }

    public Result(MyClass myClass)
    {
        Success = true; 
        MyClass = myClass;
    }

    public Result(string error)
    {
        Error error;
    }
}

If you are concerned that other programmers could confuse the constructors, consider XML comments, or possibly keeping Result as a baseclass, and create SuccessResult and FailedResult subclasses like so:

public abstract class Result
{
    public abstract bool Success {get;}
}

public class SuccessResult : Result
{
    public override bool Success { get {return true; }}
    public MyClass MyClass { get; private set; }

    public Result(MyClass myClass)
    {
        MyClass = myClass;
    }
}

public class FailedResult : Result
{
    public bool Success { get {return false};}

    public string Error { get; private set; }

    public Result(string error)
    {
        Error error;
    }
}

This successfully conforms to the Single Responsibility Principle, here a result only contains error information if an error actually occurs. From this point you could also extend Result (or FailedResult) to create an ExceptionResult as well, which would contain any Exceptions raised and providing more detailed information on the nature of the error.

As for your second part, I would leave it out. The implicit conversion gives me the impression I'm getting a boolean value of the result itself, not of its success/failure condition.