patterncsharpMinor
Expression design
Viewed 0 times
expressiondesignstackoverflow
Problem
I'm designing a Tokenizer / Parser / AST. I'm wondering if this basic idea of an expression is good and if this way of implementing Simplification is good, or should I use something like the Visitor pattern? Is implementing it in the class itself a good idea?
public abstract class Expr
{
public Expr Left;
public Expr Right;
public abstract bool CanSimplify { get; set; }
public abstract Expr Simplify();
}
public class ConstExpr : Expr
{
public double Value;
public override bool CanSimplify { get; set; } = false;
public override Expr Simplify()
{
return this;
}
public override string ToString()
{
return Value.ToString(CultureInfo.InvariantCulture);
}
}
public class AddExpr : Expr
{
public override bool CanSimplify { get; set; } = true;
public override Expr Simplify()
{
while (Left.CanSimplify)
Left.Simplify();
while (Right.CanSimplify)
Right.Simplify();
var left = Left as ConstExpr;
var right = Right as ConstExpr;
if (left == null || right == null)
return this;
return new ConstExpr {Value = left.Value + right.Value};
}
}Solution
- You have a bug in your code:
Simplify()can return a new instance, and you ignore this instance when callingLeft.Simplify()andRight.Simplify(). Also, I do not see the reason to iterate on simplification (while (Left.CanSimplify)), as it appears as each expression will only need one call to be simplified.
Exprclass exposes 2 public fields,LeftandRight. Exposing public fields is a bad practice, and I would assume there may be cases where expression can have 1 or more than 2 "parameters".
- Can you make use of Expression Trees available in .NET instead of developing your own framework? It seems like a good fit for your purpose. If not - I would still consider copying design patterns from it, like immutability of expressions and Visitor pattern for walking the expression tree.
Context
StackExchange Code Review Q#83536, answer score: 5
Revisions (0)
No revisions yet.