Abstract Factory Pattern Default Class Issue

Question

Hi, I've a class which has a method with some condition checks and append a string according to condition. I tried to refactor this code with abstract factory pattern.

My problem is related to default expression class. If none of other rules applies to related filter, then I want to apply default expression rule. However on my application; it depends the order of StandartExpression instance order in the _expressions list. If it is added first then the other rules will not be checked.

I think I missed something about that pattern. Could you help me?

This is my application code:

public class Filter
{
    private string _operator = "="; // =, !=, <,> ...

    public string Name { get; set; }
    public object Value { get; set; }
    public string Operator { get { return _operator; } set { _operator = value; } }

}

Filter filter = null;
StringBuilder whereClause = new StringBuilder();

for (int i = 0; i < array.Length; i++)
{
    filter = array[i];

    if (filter.Value.ToString().ToLower().Equals("null"))
    {
        whereClause.Append(filter.Name + " " + filter.Operator + " null ");
    }
    else if (filter.Operator.ToLower().Equals("like"))
    {
        whereClause.Append(filter.Name + ".Contains(@" + i + ")");
    }
    else
    {
        whereClause.Append(filter.Name + " " + filter.Operator + " @" + i);
    }
    whereClause.Append(" AND ");
}

Now, this is the abstract factory pattern code:

public interface ICustomExpression
{
    bool ExpressionIsValid(Filter filter);
    string GetExpressionStr(Filter filter, int index);
}


public class StandardExpression : ICustomExpression
{
    public bool ExpressionIsValid(Filter filter)
    {
        return true;
    }

    public string GetExpressionStr(Filter filter, int index)
    {
        return filter.Name + " " + filter.Operator + " @" + index;
    }
}

public class LikeExpression : ICustomExpression
{
    public bool ExpressionIsValid(Filter filter)
    {
        return filter.Operator.ToLower().Equals("like");
    }

    public string GetExpressionStr(Filter filter, int index)
    {
        return filter.Name + ".Contains(@" + index + ")";
    }
}
public class NullExpression : ICustomExpression
{
    public bool ExpressionIsValid(Filter filter)
    {
        return filter.Value.ToString().ToLower().Equals("null");
    }

    public string GetExpressionStr(Filter filter, int index)
    {
        return filter.Name + " " + filter.Operator + " null ";
    }
}

public static class ExpressionFactory
{
    private static List<ICustomExpression> _expressions = null;
    public static List<ICustomExpression> Expressions
    {
        get
        {
            if (_expressions == null)
            {
                Build();
            }
            return _expressions;
        }
    }

    private static void Build()
    {
        _expressions = new List<ICustomExpression>();
        _expressions.Add(new NullExpression());
        _expressions.Add(new LikeExpression());
        _expressions.Add(new StandardExpression());
    }
}

And that is how I used that structure on my application code:

StringBuilder whereClause = new StringBuilder();
Filter filter = null;
var array = filterList.ToArray();
for (int i = 0; i < array.Length; i++)
{
    filter = array[i];

    foreach (ICustomExpression exp in ExpressionFactory.Expressions)
    {
        if (exp.ExpressionIsValid(filter))
            whereClause.Append(exp.GetExpressionStr(filter, i));
    }


    whereClause.Append(" AND ");
}

Answer 1

Your feeling is right: there is a hidden distinction between "special" and "standard" expressions that should be made explicit for clarity.

However, in my eyes the bigger problem is that your factory is not actually handling the type selection and instantiation on its own but is delegating parts of that responsibility to its clients. Every client has to know how to work with the collection property Expressions and thus also has to know about this implicit distinction I mentioned above.

Ideally clients of the ExpressionFactory should just hand it a filter in order to get the correct ICustomExpression instance. Here's how this might look like:

public static class ExpressionFactory
{
    private static StandardExpression _standardExpression = new StandardExpression();

    private static ICustomExpression[] _specialExpressions = new []
    {
        (ICustomExpression)new NullExpression(),
        (ICustomExpression)new LikeExpression()
    };

    public static ICustomExpression GetBy(Filter filter)
    {
        var match = _specialExpressions.SingleOrDefault(e => e.ExpressionIsValid(filter));

        if (match == null)
            return _standardExpression;

        return match;
    }
}

The rules how an expression is created (or rather selected, in your case) is now only known by the factory and thus implemented in a single place.

I also made the implicit distinction between "special" and "standard" expressions explicit, so the implementation does not depend on the order in which the ICustomExpressions are added anymore.

Note that the usage of the factory now becomes more straight-forward:

for (int i = 0; i < array.Length; i++)
{
    filter = array[i];  
    var exp = ExpressionFactory.GetBy(filter);          
    whereClause.Append(exp.GetExpressionStr(filter, i));        
    whereClause.Append(" AND ");
}

Here is a working example of the refactored code: https://dotnetfiddle.net/uzVJhM

By the way: your implementation looks more like an instance of the factory method pattern.