patterncsharpMinor
Result class which wraps another object
Viewed 0 times
resultwrapsanotherwhichobjectclass
Problem
I have a result class which wraps another object if successful or an error if failure
I also am considering putting an implicit conversion to bool
So that I can do
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
}- 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);
}- Is this a silly idea?
Solution
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):
If you are concerned that other programmers could confuse the constructors, consider XML comments, or possibly keeping
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
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.
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.
Code Snippets
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;
}
}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;
}
}Context
StackExchange Code Review Q#69377, answer score: 9
Revisions (0)
No revisions yet.