Skip to content

New postfix calculator#10

Open
Palezehvat wants to merge 8 commits intomasterfrom
NewPostfixCalculator
Open

New postfix calculator#10
Palezehvat wants to merge 8 commits intomasterfrom
NewPostfixCalculator

Conversation

@Palezehvat
Copy link
Owner

Ещё 1 пулреквест

Comment on lines 7 to 10
void AddElement(double value);

// Remove element in stack and return deleted item
(bool, double) RemoveElement();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это обычно Push и Pop

namespace StackCalculator;

// Interface for the stack
interface IOperationsWithStack

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это не операции со стеком, это и есть стек

Console.WriteLine("Enter an example in the postfix form");
var stringWithExpression = Console.ReadLine();

PostfixCalculator calculator = new PostfixCalculator();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
PostfixCalculator calculator = new PostfixCalculator();
var calculator = new PostfixCalculator();

namespace StackCalculator;

//Standart stack
abstract public class Stack : IOperationsWithStack

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Этот класс, кажется, не нужен, потому что каждому настоящему стеку всё равно придётся переопределить все операции

namespace StackCalculator;

// Calculator that counts algebraic expressions in postfix form
public class PostfixCalculator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

У него нет никаких причин быть не статическим — у него всё равно нет своего состояния. Вот если бы стек Вы принимали в конструктор и запоминали, то да. Но непонятно, надо ли это.

stackArray = new double[sizeStack];
}

public bool ChangeStackSize(int size)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Этому методу ни к чему быть public:

  • мы всё равно про него не знаем, потому что работаем со стеком через интерфейс, где этого метода нет, так что и вызвать его не сможем;
  • изменение размера стека — внутреннее дело стека, никто извне не должен ему мочь это указать;
  • этот метод сильно намекает на то, как стек устроен внутри — нарушение принципа сокрытия деталей реализации.

// Stack implemented on list
public class StackList : Stack
{
private StackElement headStack;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Надо сделать так, чтобы nullability не ругалась

public class Tests
{
private const double delta = 0.0000000000001;
PostfixCalculator calculator;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private, и надо пустую строку после

[TestCaseSource(nameof(Stacks))]
public void TheCalculatorShouldWorkCorrectlyToReturnTheCorrectValueOnASimpleExample(Stack stack)
{
Setup();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Обратите внимание на [SetUp] перед определением этого метода, и посмотрите, что делает этот атрибут

{
Setup();
var (isCorrect, number) = calculator.ConvertToAResponse("1 2 +", stack);
Assert.IsTrue(isCorrect && number == 3);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Надо разбить на два Assert.That

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments