0

I have a collection of classes which model Logical(Boolean) Expressions.

There is a base abstract class LogicalExpression, and derived classes : UnaryLogicalExpression, BinaryLogicalExpression, LogicalParameter, LogicalValue.

Some code samples

public abstract class LogicalExpression
{
    public string[] parameterNames;
    public abstract Boolean Evaluate(bool[] values);
}

public class BinaryLogicalExpression : LogicalExpression
{
    public LogicalExpression LeftSide { get; set; }
    public LogicalExpression RightSide { get; set; }
    public LogicalOperator Operator { get; set; }

    public override bool Evaluate(bool[] values)
    {
        bool leftSideResult = LeftSide.Evaluate(values);
        bool rightSideResult = RightSide.Evaluate(values);
        switch (Operator)
        {
            case LogicalOperator.And:
                {
                    return leftSideResult && rightSideResult;
                }
            case LogicalOperator.Or:
                {
                    return leftSideResult || rightSideResult;
                }
            case LogicalOperator.Xor:
                {
                    return leftSideResult ^ rightSideResult;
                }
            default:
                {
                    throw new InvalidOperationException("Operator not valid!");
                }
        }
    }
    public BinaryLogicalExpression(LogicalOperator Operator, LogicalExpression LeftSide, LogicalExpression RightSide)
    {
        this.Operator = Operator;
        this.LeftSide = LeftSide;
        this.RightSide = RightSide;
    }
}

public class LogicalParameter : LogicalExpression
{
    public int Index { get; set; }
    public string Name { get; set; }

    public override bool Evaluate(bool[] values)
    {
        try
        {
            return values[Index];
        }
        catch (IndexOutOfRangeException ex)
        {
            throw new IndexOutOfRangeException("The array of values must match the parameter list length!");
        }
    }
    public LogicalParameter(int Index, string Name)
    {
        this.Name = Name;
        this.Index = Index;
    }
    public override string ToString()
    {
        return Name;
    }
}

The issue/dillema here is the LogicalParameter class. Right now it's very simple, it represents an atomic boolean variable, but I need it do do more.

I want to have a LogicalExpression that can have more complex parameters (not just atomic boolean variables).

For example, I have another class Segment which implements an interface ISegment. Any type of ISegment can check if a profile is associated with that segment.

public interface ISegment
{
    bool CheckFor(Guid ProfileId);
}

Now I want to put these two together. I want to have a LogicalExpression that can have segments instead of atomic bool variables and then I want to evaluate it by providing a profileId.

What's the best way of doing this ?

I could change the LogicalExpression class, override Evaluate() so that it takes a profileId instead of an array of values and then make my Segment class(es) inherit from LogicalParameter. But this is just horrible, there's no separation of concerns.

My LogicalExpression class doesn't need to know about Segments right ? So how should I approach this ? I have a feeling it's something very basic which I'm missing.

1

There are a couple of language implementation concepts that could help you here. The first is the concept of an environment. Currently, your expression hierarchy also keeps track of variable names etc. There are aspects where this might be useful, but the environment is usually extracted into a different object. This environment maps variable names to values. When we evaluate an expression, the environment object is passed as an argument to the evaluation function. Now, your expression nodes don't need to keep track of an array of parameter names, and do not have to use error-prone indices rather than names (indices are better if you compile your expression tree to some byte code, but are otherwise difficult to keep correct if whoever creates a LogicalParameter has to keep track of used indices).

When a new environment is created, we want to make sure that it contains all required variables. You check that during evaluation if and index is out of range, but we can do better: a method FreeVariables that returns the variables in an expression. This is an array of a single element for a LogicalParameter, and the concatenation of the left side's free variables with the right side's free variables for a binary expression. Given a list of free variables, the constructor of an environment object can enforce that all variables are provided. Note that your expression objects should be immutable so that the list of free variables always stays correct.

Next, you are trying to provide variables of different types: bool and ProfileId. We therefore need a kind of type system in our expression evaluator and in our environment. There are two approaches: static and dynamic typing. With dynamic typing, we might create our own types that represent values in the system, e.g. an interface IValue that is implemented by MyBool and MyProfileId etc. so that the environment can just contain a mapping of names to IValue objects (in C# or other languages with a unified type hierarchy this can be also done by using object as the type root). The Evaluate method also returns IValue objects rather than bool. Each operation that uses an IValue would first have to cast the value to the expected concrete type, or issue a runtime type error. This is fairly straightforward to implement, but it is possible to get more safety.

If we want static typing, we notice that some expressions might return bool and others might return ProfileId. This can be modelled via generics. We'd now have:

interface Expression<T> {
  T Evaluate(Environment);
}

class BinaryExpression : Expression<bool> {
  BinaryExpression(Operator op, Expression<bool> left, Expression<bool> right) { … }
  override bool Evaluate(Environment) { … }
}

class Variable<T> : Expression<T> {
  Variable(string name) { … }
  override T Evaluate(Environment) { … }
}

class SegmentExpression : Expression<bool> {
  SegmentExpression(ISegment segment, Expression<ProfileId> value) { … }
  override bool Evaluate(Environment env) {
    return Segment.CheckFor(Value.Evaluate(env));
  }
}

So essentially, we use generics to piggy-back on the type system of the host language. This works all right as long as the type system is sufficiently expressive. The Environment might still have to store the values as a dynamic type, but we can check that all types match before evaluation. Any runtime casts are then guaranteed safe. While this is implemented as dynamic typing, this does effectively provide static typing to your language (but the same is true for any other language since processor instruction sets are typically untyped).

  • I like the environment idea. I think it's easier if the segments implement ILogicalExpression and I get the profileId from the environment. – Adrian Buzea Feb 26 '16 at 12:51
1

I personally think you're missing a layer. You should (perhaps) have a service/module, taking list of ISegment variables, process the list in this service class and pass the processed result to a LogicalEpxression implementation (still taking the bool variables as a parameter).

It seems like you're programming in C#, if that is the case, other possibility is turning LogicalExpression into a generic class and having specialized implementations, but I personally do not think that is going to solve anything but lead to even tighter coupling.


Speaking of separation of concerns, the switch/case scenario in your BinaryLogicalExpression should be actually refactored by adding an Evaluate method to your LogicalOperator interface, taking two LogicalExpression parameters (left and right side) and you should provide implementations to do the right operation.

  • Yes, another layer would do the job, and would be relatively easy to implement. LogicalOperator is an Enum not an interface, so the switch statement is ok as it is for now. – Adrian Buzea Feb 26 '16 at 15:33

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.