2

I have a situation where I need to model objects that don't share common attributes but represent same logical entity. Now, based on their type they will have different attributes (properties). To keep an example code simple & easy to understand, let's say we have Valid & Invalidvalidation results. When validation result is invalid it exposes error message, otherwise it doesn't expose anything.

These are the validation result object models:

public interface IValidationResult
{ }

public sealed class Valid : IValidationResult
{
    private Valid()
    { }

    public static Valid Instance { get; } = new Valid();
}

public sealed class Invalid : IValidationResult
{
    public string ErrorMessage { get; }

    public Invalid(string errorMessage)
    {
        ErrorMessage = errorMessage;
    }
}

In a way I need a discriminated union which is not supported in C# yet.

The usage:

static IValidationResult Validate(string name)
{
    if(string.IsNullOrWhiteSpace(name))
        return new Invalid("Name can't be null or empty string");

    return Valid.Instance;
}

static async Task Main(string[] args)
{
    var validationResult = Validate("Michael");
    switch (validationResult)
    {
        case Valid _:
        {
            break;
        }
        case Invalid invalidResult:
        {
            Console.WriteLine($"Invalid name: {invalidResult.ErrorMessage}");
            break;
        }
    }
}

As can been seen, I'm making use of C#'s patter matching to make a decision on program flow based on type of validationResult.

While I appreciate that this is a readable piece of code, it doesn't look like OO code to me. So, I went ahead and created 2nd version of this, which is more OO to me, but is more code and not so elegant (IMO).

public interface IValidationResult2
{
    void UseResult(Action<string> useValidationResultAction);
}

public sealed class Valid2 : IValidationResult2
{
    private Valid2()
    { }

    public static Valid2 Instance { get; } = new Valid2();
    public void UseResult(Action<string> useValidationResultAction)
    { }
}

public sealed class Invalid2 : IValidationResult2
{
    public string ErrorMessage { get; }

    public Invalid2(string errorMessage)
    {
        ErrorMessage = errorMessage;
    }

    public void UseResult(Action<string> useValidationResultAction)
    {
        useValidationResultAction(ErrorMessage);
    }
}

And the usage:

static IValidationResult2 Validate2(string name)
{
    if (string.IsNullOrWhiteSpace(name))
        return new Invalid2("Name can't be null or empty string");

    return Valid2.Instance;
}

static async Task Main(string[] args)
{
    var validationResult2 = Validate2("Michael");
    validationResult2.UseResult(errMsg => Console.WriteLine($"Invalid name: {errMsg}"));
}

As I said while this 2nd approach looks more OO to me, however, I feel like this is less readable/elegant.

I can think of a 3rd option, where validationResult would allow to configure program flow based on result (whether it is valid or invalid), but that would also fall under cons of 2nd approach. Without full implementation, this is the how 3rd option usage would look like:

static async Task Main(string[] args)
{
    var validationResult3 = Validate3("Michael");
    validationResult3
        .WhenValid(_ => { })
        .WhenInvalid(errMsg => { Console.WriteLine($"Invalid name: {errMsg}"); })
        .Execute();
}

Which one should be preferred & why ? Would you consider it a code smell when you see pattern matching in OO code ?

  • Who or what is going to process that validation result? Can't it just be text or a piece of XML that has everything there is to know? In either case, a simple string? – Martin Maat Jul 21 '18 at 18:25
2

Not everything has to be perfectly object-oriented, even if you have to call nearly everything a class in C#.

Your first design is a reasonable approximation of an union in C#. You could make it slightly safer by preventing other implementations of the validation result interface, e.g. by using a base class with an internal constructor.

Your second design uses a radically more complex implementation by using functional techniques. This complexity can pay off if this allows you to use combinators or common patterns, or when it enables a whole ecosystem of applications – see e.g. LINQ. Here, it only saves you an if-statement.

You might note that your result type is either OK or an error string, which could also be represented (slightly less clearly) with a nullable type such as string or a custom nullable Error. It is of course possible to avoid nulls and return a special instance instead. However, you do not need full pattern matching with a switch if there are only two cases (for one of which you will do nothing).

E.g. if we define

public sealed class Result {
  private Result(string msg) { ErrorMessage = msg; }
  public static Result Valid => new Result(null);
  public static Result Invalid(string msg) => new Result(msg);

  public string ErrorMessage { get; }
  public bool IsValid => ErrorMessage == null;
}

We can use a validationResult like

if (!validationResult.IsValid)
  Console.WriteLine($"Invalid name: {validationResult.ErrorMessage}");

or

if (validationResult.ErrorMessage is string msg)
  Console.WriteLine($"Invalid name: {msg}");
  • Thank you for answer! I'm usually using Result class you showed here, but I didn't include it in the post as I had more complex cases in mind than just OK/Error - my bad choosing poor analogy. Probably better example would have been Shapes - e.g. Rectangle, Cube, Circle etc... Would you say that in this case where models are more than 2 (and possibly even more - they might grow in the future in terms of supported shapes to operate on) you would go with pattern matching ? – Michael Jul 21 '18 at 13:50
  • 2
    @Michael Yes with more than 2 choices pattern matching becomes more attractive, even though it is anti-OOP. A more typesafe alternative (because it fails to compile with missing cases) could be the visitor pattern, but that's a lot of extra code. If you are doing a switch over object types, you should consider carefully whether the cases shouldn't be methods of those objects instead. – amon Jul 21 '18 at 14:07
1

In your concrete example pattern matching can be implemented with polymorphic features of OO.

public interface IValidationResult
{
    void Print();
}

public class FailedValidation : IValidationResult
{
    private readonly string _errorMessage;

    public FailedValidation(string errorMessage) => _errorMessage = errorMessage;

    public void Print() => Console.WriteLine(_errorMessage);
}

public class SuccessValidation : IValidationResult
{
    public void Print() => {}; // do nothing
}

// Validation
public class Validator
{
    public IValidationResult Validate(string message)
    {
        if (string.IsNullOrEmpty(message))
        {
            return new FailedValidation("Invalid name");
        }

        return new SuccessValidation();
    }
}

Then your man code will look simple and without extra condition logic.

static void Main(string[] args)
{
    var name = args[0];
    var result = new Validator().Validate(name)
    result.Print();
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.