Does my code violates Single Responsibility Principle?

Question

I'm making a console calculator as a test project. Main feature of my realisation of it is that i made digits and operators classes inherited from ITerm interface:

public interface ITerm
{
    Object Value { get; }
}

Now, i'm inheriting it in IOperand and IOperator interfaces and using these intefaces in further calculations via reverse poland notation.

And now, i've been told that using this Object-type property both to keep numbers and operators is violation of Single Responsibility Principle.

private ITerm CalculatePostfixExpression(IEnumerable<ITerm> input)
{
    var tempResult = new Stack<ITerm>();
    foreach (var term in input)
    {
        if (term is IOperand)
        {
            tempResult.Push(term as IOperand);
        }
        if (term is IOperator)
        {
            tempResult.Push(ProceedOperation(term as IOperator, tempResult));
        }
    }
    return tempResult.Peek();
}

This is how i am handling calculation. So there are two questions: 1. Is there some defects in my idea about storing both operands and operators in Object variable? 2. Is there some ways to improve my code? I'm considering using visitor pattern in CalculatePostfixExpression method now.

You should post it here: http://codereview.stackexchange.com/ — Jeroen van Langen, Nov 08 '16 at 21:56
thank you for guiding me, should i delete my question there after posting on codereview? — Occam's chainsaw, Nov 08 '16 at 22:56

score 2 · Accepted Answer · answered Nov 08 '16 at 22:07

It's definitely not the most obvious approach imho.

It is also not very clear to me what you're storing in the Value property ?

The fact that you have 2 interfaces IOperand and IOperator that both derive from the same base interface ITerm, while those IOperand and IOperator do not have a is-a relation is imho a code smell at least.

This means that IOperand and IOperator are not interchangeable although they're both ITerm instances, which is a violation of the Liskov Substitution principle (one of the SOLID principles).

I have created a similar console-test project myself (reverse polish calculator console-app) a few weeks ago, which you can find on github: https://github.com/fgheysels/Calculator

Thanks a lot, i should look at your code. Value property holds, ehm, content of term like "128" or "+" — Occam's chainsaw, Nov 08 '16 at 22:56

Does my code violates Single Responsibility Principle?

1 Answers1