-3
\$\begingroup\$

Hope, it's my last question about current project. Yeah, it's still about respecting SOLID-principles.

And it's still about calculator, so i've got realization of ITerm interface:

public interface ITerm : IStackManipulator
{
    Object Value { get; }
    /// <summary>
    /// Term type. 0 for operands, 1 for operators, 2 for brackets
    /// </summary>
    int ValueType { get; }
}

So, my interviewer requires that clients of this interface (classes which uses ITerm objects) should always know, what current term is: operand, operator or smth else (e.g. brackets).

The easiest (and currently implemented way) is to create this int ValueType { get; }property and to set it directly in class constructor. Example of my code is here:

public class Addition : IOperator //IOperator implements ITerm
{
    public int Priority { get; }
    public object Value { get; }
    public int ValueType { get; } = 1;//etc

And it's violates Single-Responsibility principle :C

As i know from reading titles about SRP-Validation the easiest way to refactor this, is to create something like ValueTypeGeneratorclass and call for something like ValueTypeGenerator.Generate() in constructor (i know about DI, but i want to simplify code there) but it looks pretty ...weird and i really don't like to use this way in my case.

So, i'm asking for an another advice, may be there is some other ways to improve that?

\$\endgroup\$
  • 2
    \$\begingroup\$ Anyway... all I have to say about the code is why are you using ints for Priority and ValueType rather than enums? \$\endgroup\$ – 404 Feb 24 '16 at 23:07
  • \$\begingroup\$ Priority should be int as i think, and ValueType is just an experiment, so i'll change it into enum soon enough \$\endgroup\$ – Occam's chainsaw Feb 24 '16 at 23:10
  • \$\begingroup\$ @Occam'schainsaw FYI, I contacted the community team to merge your two accounts \$\endgroup\$ – janos Feb 25 '16 at 8:04
  • 2
    \$\begingroup\$ It's really difficult to review isolated snippets like this that don't really do anything. You should just post your working code. \$\endgroup\$ – 200_success Feb 27 '16 at 3:01
2
\$\begingroup\$

There is not enough here to understand the question. I read your original post in StackOverflow and I'm thinking you need to step back and reconsider the design.

There is too much unnecessary abstraction. Everything is "decomposing" into meaningless object things through the chain of interfaces. Think about the "calculator domain" and capture those things.

Drop all that interface stuff

Forget this for now because we need to focus on the big picture design. As design and code evolve then interface needs, if any, will become more apparent.

I have sketched out my thoughts below. It is not a complete solution. But there are no "items" or "objects" or "value types".

You may get into issues about integers vs. floating point for example but first identify the things in the "RPN calculator domain"

Operators, Operands, and Expressions

  • Operators of course are '+', '-', '*' and so forth.

    • This could be char[] operators = { '+', '-', '*' ... }.
    • We are defining all the operators here. If it is not in this list, it is not an operator.
    • I think of parenthesis "(" and ")" - what I assume you called brackets - as operators. If you want to classify them as something else that's OK.
  • Operands are the numbers involved in the calculation.

    • I think these will be input as strings as part of an expression and be cast to numbers when necessary.
  • An Expression is the calculation as entered.

Reverse Polish Notation

At the core of RPN is a stack. Use the .net stack class

I expect this will be a stack of strings - the pieces of a parsed expression.

The algorithm

Figure out the specifics on how to implement RPN. For example:

  1. user enters "2 2 +"
  2. Parse the 1st "2". That's an operand, push it on the stack
  3. Parse 2nd "2". That's an operand, push it on the stack
  4. Parse "+". That's an add operator.
    • Pop stack twice - these better be operands! -
    • and add them.
  5. push result onto the stack.
\$\endgroup\$
  • \$\begingroup\$ Thanks a lot for your answer! Unfortunately, i read your post just now, so i'm sorry for so late feedback. I'll consider your advice about re-structuring my app architecture and all other advices. \$\endgroup\$ – Occam's chainsaw Mar 4 '16 at 6:35

Not the answer you're looking for? Browse other questions tagged or ask your own question.