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?

• svick: Why do you want to use this instead of exceptions? What kind of operations would use this?
• StingyJack: Whenever there is more useful information to communicate out to the caller than an exception can handle (like for something that can partially succeed) or when involving some generic proxy (like something calling across a network boundary) that simplifies outcomes to pass + result, or fail. Thinking about it now, returning null is never really a favorable result for exposed methods as it just means either more time guarding against it in code or more time debugging the issues that crop up.

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.

• dav_i: Thanks for your feedback. I've opted for the factory methods as it doesn't make sense for there to be an Error with a MyClass and for Success to be true for a Result with an Error present and visa versa. The use of factory methods here guarantees the state.
• Nick Udell: So would two constructors, I shall edit my answer to show how.
• dav_i: Oh I see what you mean, still have a private parameterless constructor but have one with string and one with MyClass?
• Nick Udell: At that point you do not need the private parameterless constructor because the compiler will not auto-generate a public parameterless constructor (because other constructors exist).
• dav_i: WRT having derived classes, how would you end up consuming them? Having chains of if (result is ErrorResult) else if ... seems inelegant.