Am I violating the Single Responsibility Principle with this AST Node of mine?

Question

We are building an interpreter for a language as part of the requirements in one of our undergraduate course. Part of the project involves writing a lexer and a parser. The parser generates an AST. Obviously, the AST would have nodes.

One of the nodes of that AST is the ArithmeticOperationNode. This node will be a node containing any operations involving arithmentic. The code below is a minimized version of the node.

package ds.ast;

import exceptions.CompilerException;
import exceptions.RuntimeException;

public class ArithmeticOperationNode extends OperationNode
{
    public ArithmeticOperationNode(String operation, Computable operator1, Computable operator2, int startingLine, int startingColumn)
    {
        super(operation, operator1, operator2, startingLine, startingColumn);
    }

    public Value compute() throws RuntimeException, CompilerException
    {
        ...
    }
}

NOTE: OperationNode implements an interface called Computable.

compute() will be responsible for performing the arithmetic operation between operator1 and operator2. operator1 and operator2 may be another arithmetic expression. The function will be called when we traverse the AST during execution. There are a lot of nodes that follow the same design, i.e. having a compute() function.

Am I violating the Single Responsibility Principle with this? I have a feeling that I am doing so.

Answer 1

There do seem to be some conflation of responsibilites in that you have syntax level information (operation indicated as a string, line nr etc.) together with the semantic operation. I suspect compute() will end up having code similar to this (pseudocode):

if operation=="+":
  return a + b
if operation=="-":
  return a - b
...

Here the interpretation of the syntax is conflated with the semantics of the evaluation. The AST shouldn't really depend on the concrete syntax, which is why it is called an Abstract Syntax Tree. Instead the parser could generate AST classes like AddExpression, SubstractionExpression etc. which would only contain the semantic operation in compute() but not have any knowledge of the underlying syntax.

Answer 2

Yes you are. It may be because of your naming. You call your object ArithmeticOperation, already suggesting it does something. While at parse time it is still just an expression. Later in your question you do use that word (expression) which is really the better word in an AST context. You would not feel the need to add a Compute() method if the thing were called ArithmeticExpression.

I think the better route would be to recognize expressions rather than operations. You have two kinds: Unary and binary. As goes for operators: unary operators (like not) and binary operators (and, plus).