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

Condition class

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

Problem

Up-to-date version: Countable and uncountable sets in .NET (IEnumerable and Predicate).

Here is my own predicate class; it is equipped with some operators. Demo:

using static BusinessObjects;
using static Console;

class Program
{
    static void Main(string[] args)
    {
        WriteLine(Sell("John Doe", "test@example.com", 10000));
        WriteLine(Sell("John Doe", "test@example.com", 100000));
        WriteLine(Sell("John Doe", "test@example.com", 150000));
    }

    static string Sell(string name, string email, int income)
    {
        if (name == NullOrWhiteSpace || email != ValidEmail)
            throw new Exception("Bad, bad customer.");

        if (income == LowIncome)
            return "Sell car.";

        var middleIncome = !LowIncome && !HighIncome; // magic!!!
        if (income == middleIncome)
            return "Sell home.";

        return "Sell big.";
    }
}


Where business definitions are (verbose, verbose C#):

static class BusinessObjects
{
    public static readonly Condition NullOrEmpty = 
        new Condition(string.IsNullOrEmpty);
    public static readonly Condition NullOrWhiteSpace = 
        new Condition(string.IsNullOrWhiteSpace);

    public static readonly Condition ValidEmail = 
        new Condition(new EmailAddressAttribute().IsValid);

    public static readonly Condition LowIncome = 
        new Condition(i => i  HighIncome = 
        new Condition(i => i > 140000);
}


Library class:

```
public class Condition
{
public Condition(Predicate predicate)
{
Predicate = predicate;
}

Predicate Predicate { get; }

public static bool operator ==(Condition left, T right) =>
left.Equals(right);

public static bool operator ==(T left, Condition right) =>
right.Equals(left);

public static bool operator !=(Condition left, T right) =>
!left.Equals(right);

public static bool operator !=(T left, Condition right) =>
!right.Equals(left);

publi

Solution

Honestly I don't see the advantage of Condition class over a simpler Predicate delegate (eventually with few extension methods to make combinations easier).

Few issues that makes me perplex (at best):

  • You're abusing Equals() to check something that is not equality. This is especially bad because it's absolutely unclear unless users read your Condition implementation.



  • Equality must be commutative and transitive and your actual implementation is not (if "adriano@example.com" == ValidEmail && "dmitry@example.com" == ValidEmail however "adriano@example.com" != "dmitry@example.com").



  • You're throwing NotSupportedException for GetHashCode() however it's used by many .NET Framework classes, what if your users will keep your conditions inside an HashSet?



In short you're adding a class Condition to introduce few handy methods (which will be probably never called in real-world) but with a semantic that will confuse everyone will read your code. Think if button to flush toilet was labeled "mirror light". Confusing, right?

Let's go to your test code, is this:

if (name == NullOrWhiteSpace || email != ValidEmail)
    throw new Exception("Bad, bad customer.");


More clear or shorter than this (using C# 6 using static syntax):

if (IsNullOrWhiteSpace(name) || !IsValidEMail(email))
    throw new ArgumentException("...");


I don't think so and it integrates smoothly into existing code. First principle: don't make your users astonished! You still need some syntactic sugar for combinations:

public static class PredicateExtensions {
    public static Predicate Not(this Predicate rhs) {
        return x => !rhs(x);
    }

    public static Predicate And(this Predicate lhs, Predicate rhs) {
        return x => lhs(x) && rhs(x);
    }

    public static Predicate Nand(this Predicate lhs, Predicate rhs) {
        return x => !lhs(x) && !rhs(x);
    }
}


Used like this:

var middleIncome = HighIncome.Nand(LowIncome); // No magic!


Of course you can drop extension methods if you prefer a plainer syntax:

var middleIncome = Nand(HighIncomem, LowIncome); // Even less magic!


In general what I feel to suggest is to don't try to abuse C# syntax to make your business logic shorter. First of all because shorter code isn't necessarily better (whichever meaning you give to better), if you need a simple (or closer to domain) syntax then create a Domain Specific Language to describe your business logic at high level.

I do not mean that you may not need a Condition class at all. If you're modeling your domain specific constraints in C# it can be pretty handy to have a specific class instead of a generic delegate Predicate but I wouldn't override the comparison operators, in no way a == b can be a ⊢ b.

A possible implementation should also introduce a base non-generic Condition class to allow combinations - if providing required parameters - for conditions with different Ts (this is helpful only if your Condition class also holds its own data.)

Code Snippets

if (name == NullOrWhiteSpace || email != ValidEmail)
    throw new Exception("Bad, bad customer.");
if (IsNullOrWhiteSpace(name) || !IsValidEMail(email))
    throw new ArgumentException("...");
public static class PredicateExtensions {
    public static Predicate<T> Not(this Predicate<T> rhs) {
        return x => !rhs(x);
    }

    public static Predicate<T> And(this Predicate<T> lhs, Predicate<T> rhs) {
        return x => lhs(x) && rhs(x);
    }

    public static Predicate<T> Nand(this Predicate<T> lhs, Predicate<T> rhs) {
        return x => !lhs(x) && !rhs(x);
    }
}
var middleIncome = HighIncome.Nand(LowIncome); // No magic!
var middleIncome = Nand(HighIncomem, LowIncome); // Even less magic!

Context

StackExchange Code Review Q#131935, answer score: 13

Revisions (0)

No revisions yet.