Stack calculator by MinyazevR · Pull Request #4 · MinyazevR/Homework-2-sem

MinyazevR · 2022-03-04T20:33:28Z

No description provided.

yurii-litvinov · 2022-04-10T07:53:10Z

+using System;
+using Stack;
+
+namespace StackCalculator;


namespace лучше в самом начале писать

yurii-litvinov · 2022-04-10T07:53:48Z

+/// </summary>
+public class Calculator
+{
+    private IStack<float> Stack = new StackOnLists<float>();


По условию, в коде класса «Стековый калькулятор» не должно быть ни одного упоминания конкретных реализаций стека, даже если очень хочется.

yurii-litvinov · 2022-04-10T07:55:09Z

+    /// <returns>Expression value</returns>
+    public float CountTheExpressionInPostfixForm(string[] inputString)
+    {
+        int number = 0;


Слишком рано объявлен. Надо его объявить в месте инициализации: if (int.TryParse(inputString[i], out int number))

yurii-litvinov · 2022-04-10T08:10:43Z

+/// <summary>
+/// A class representing the stack interface
+/// </summary>
+abstract public class IStack<T>


Это должен был быть интерфейс, а не абстрактный класс

yurii-litvinov · 2022-04-10T08:12:26Z

+    /// Function that returns the number of elements in the stack
+    /// </summary>
+    /// <returns>Number of elements in stack</returns>
+    abstract public int ReturnNumberOfElements();


Идеологически правильнее NumberOfElements();, "Return" тут несколько избыточно.

yurii-litvinov · 2022-04-10T08:42:07Z

+            numberOfElements--;
+            return value;
+        }
+        return default(T);


Suggested change

return default(T);

return default;

yurii-litvinov · 2022-04-10T08:42:28Z

+        if (head == null)
+        {
+            throw new StackException("Stack is empty");
+        }
+        return head == null ? default(T) : head.Value;


yurii-litvinov · 2022-04-10T08:42:59Z

+    private static IEnumerable<TestCaseData> Stacks
+    => new TestCaseData[]
+    {
+        new TestCaseData(new StackOnArray<int>()),
+        new TestCaseData(new StackOnLists<int>()),
+    };


Suggested change

private static IEnumerable<TestCaseData> Stacks

=> new TestCaseData[]

{

new TestCaseData(new StackOnArray<int>()),

new TestCaseData(new StackOnLists<int>()),

};

private static IEnumerable<TestCaseData> Stacks

=> new TestCaseData[]

{

new TestCaseData(new StackOnArray<int>()),

new TestCaseData(new StackOnLists<int>()),

};

yurii-litvinov · 2022-04-10T08:45:00Z

+    public void CheckTopOfTheStackAfterPush(IStack<int> stack)
+    {
+        stack?.Push(1);
+        Assert.AreEqual(stack?.ReturnTopOfTheStack(), 1);


Наоборот, сначала Expected, затем Actual. Иначе если тест не пройдёт, сообщение об ошибке будет лживым.

yurii-litvinov · 2022-04-10T08:45:24Z

+    [TestCaseSource(nameof(Stacks))]
+    public void CheckTopOfEmptyStack(IStack<int> stack)
+    {
+        var exception = Assert.Throws<StackException>(() => stack?.ReturnTopOfTheStack());


"?" тут не нужен, стек гарантированно не null

--Each class in a separate file --Changed tests --The stack calculator class knows nothing about the stack implementation

yurii-litvinov

Неаккуратно, но по существу всё ок, зачтена.

yurii-litvinov · 2022-05-30T12:58:03Z

+    }
+
+
+    [TestCaseSource(nameof(Stacks))]


Suggested change

}

[TestCaseSource(nameof(Stacks))]

}

[TestCaseSource(nameof(Stacks))]

yurii-litvinov · 2022-05-30T12:58:28Z

+/// <summary>
+/// A class for testing a stack calculator
+/// </summary>
+public class TestsStackCalculator


Suggested change

public class TestsStackCalculator

public class StackCalculatorTests

Так каноничнее

yurii-litvinov · 2022-05-30T12:59:00Z

+    public void ShouldThrowsDivideByZeroExceptionWhenDivideByZero(IStack<float> stack)
+    {
+        string[] firstArray = { "123", "0", "/" };
+        Assert.Throws<System.DivideByZeroException>(() => stackCalculator?.CountTheExpressionInPostfixForm(firstArray, stack));


Вообще, калькулятор должен принимать строку, а не массив, но ладно :)

yurii-litvinov · 2022-05-30T12:59:18Z

+    public void ShouldThrowsInvalidCharacterExceptionWhenExpressionContainsInvalidCharacter(IStack<float> stack)
+    {
+        string[] firstArray = { "123", "0", "a" };
+        Assert.Throws<InvalidCharacterException> (() => stackCalculator?.CountTheExpressionInPostfixForm(firstArray, stack));


Suggested change

Assert.Throws<InvalidCharacterException> (() => stackCalculator?.CountTheExpressionInPostfixForm(firstArray, stack));

Assert.Throws<InvalidCharacterException>(() => stackCalculator?.CountTheExpressionInPostfixForm(firstArray, stack));

yurii-litvinov · 2022-05-30T12:59:27Z

+    }
+
+
+    [TestCaseSource(nameof(Stacks))]


Suggested change

}

[TestCaseSource(nameof(Stacks))]

}

[TestCaseSource(nameof(Stacks))]

yurii-litvinov

Сам калькулятор со стеками ещё всё-таки надо немного поправить

yurii-litvinov · 2022-06-01T13:03:20Z

+    /// Function for counting expressions in postfix form
+    /// </summary>
+    /// <returns>Expression value</returns>
+    public float CountTheExpressionInPostfixForm(string[] inputString, IStack<float> stack)


Нет-нет, хочется, чтобы калькулятор принимал строку, а не массив строк

yurii-litvinov · 2022-06-01T13:04:13Z

+            catch (StackIsEmptyException)
+            {
+                throw;
+            }


Если вы думаете, что ваша жизнь бессмысленна, посмотрите на эти строки кода :)

yurii-litvinov · 2022-06-01T13:06:19Z

+/// </summary>
+public class IncorrectExpressionException : Exception
+{
+    public IncorrectExpressionException() : base() { }


Это тоже нечто ненужное. Если в классе не объявлен ни один конструктор, компилятор сам генерирует как раз это.

yurii-litvinov · 2022-06-01T13:07:18Z

+    {
+        if (values != null && numberOfElements == values.Length)
+        {
+            Array.Resize(ref values, values.Length + 20);


Лучше увеличивать сразу вдвое. Иначе как у Вас операция Push работает в среднем за линию, а если стек растёт вдвое, то в среднем за константу.

yurii-litvinov · 2022-06-01T13:08:03Z

+        }
+
+        numberOfElements++;
+        if (values == null)


Кажется, nullability-анализ гарантирует, что values не null, так что это довольно бессмысленная проверка

yurii-litvinov · 2022-06-01T13:08:49Z

+        }
+
+        T topOfSTack = values[numberOfElements - 1];
+        Array.Clear(values, numberOfElements - 1, 1);


Это хитрый способ записать values[numberOfElements - 1] = default;

MinyazevR added 2 commits February 28, 2022 19:03

Writing basic stack functions, writing tests

3e33fb1

Rollback of commits due to their uninformativeness

8c940cf

MinyazevR force-pushed the StackCalculator branch from 3125d75 to 8c940cf Compare April 2, 2022 19:39

yurii-litvinov suggested changes Apr 10, 2022

View reviewed changes

MinyazevR added 5 commits April 10, 2022 21:46

--Changed working with exceptions

df38baa

--Each class in a separate file --Changed tests --The stack calculator class knows nothing about the stack implementation

Changed function names in tests

ffa5899

Removed unnecessary messages in exceptions

5cefc28

Removing unnecessary code

8f6905f

Changing the code by style guide

33eba1a

yurii-litvinov approved these changes May 30, 2022

View reviewed changes

yurii-litvinov suggested changes Jun 1, 2022

View reviewed changes

MinyazevR added 3 commits June 2, 2022 00:44

add yml

0dfd251

fixed yml

f909b96

rename yml

b8ce61c

	public class TestsStackCalculator
	public class StackCalculatorTests

	Assert.Throws<InvalidCharacterException> (() => stackCalculator?.CountTheExpressionInPostfixForm(firstArray, stack));
	Assert.Throws<InvalidCharacterException>(() => stackCalculator?.CountTheExpressionInPostfixForm(firstArray, stack));

Conversation

MinyazevR commented Mar 4, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yurii-litvinov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yurii-litvinov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants