Calculator by AnNyiiik · Pull Request #4 · AnNyiiik/HWSecondTerm

AnNyiiik · 2023-03-09T22:36:08Z

No description provided.

…ated class which ran method calculate (calculate an postfix expression's value), added test class

viktoriia-fomina · 2023-03-12T08:01:00Z

+public interface IStack
+{
+    public Tuple<bool, double> Calculate(string expression);
+}


Стек - это структура данных, calculate она не умеет. Зато умеет push и pop :) Переделайте, методы интерфейса, пожалуйста

viktoriia-fomina · 2023-03-12T08:03:07Z

+        {
+            return new Tuple<bool, double>(false, 0);
+        }
+        var parsedOperands = expression.Split(' ');


Можно просто Split(). Он тоже по пробелам разбивает

viktoriia-fomina · 2023-03-12T08:07:11Z

+        {
+            return new Tuple<bool, double>(false, 0);
+        }
+        return new Tuple<bool, double>(true, stack[0]);


Метода Calculate в стеке быть не должно. Стек только знает как данные складывать и отдавать: по принципу first in last out.
А вот калькулятор должен реализовывать логику выполнения арифметических операций через стек, то есть используя его методы Push и Pop

viktoriia-fomina · 2023-03-12T08:07:55Z

+
+public class StackBasedOnList : IStack
+{ 
+    private static readonly double Delta = 0.00001;


Аналогично, поля в C# со строчной буквы именуются

viktoriia-fomina · 2023-03-12T08:08:47Z

+    if (!Test.TestStackImplementation())
+    {
+        Console.WriteLine("Test has been failed");
+        return;
+    }
+    Console.WriteLine("To choose a stack based on array press 1\nTo choose a stack based on list press 2\n");
+    var isCorrect = Int32.TryParse(Console.ReadLine(), out var option);
+    if (!isCorrect)
+    {
+        Console.WriteLine("Not a number");
+        return;
+    }
+
+    if (option > 2 || option < 1)
+    {
+        Console.WriteLine("Not an option");
+        return;
+    }
+    StackCalculator calculator;
+    if (option == 1)
+    {
+        var stackArray = new StackBasedOnArray();
+        calculator = new StackCalculator(stackArray);
+    }
+    else
+    {
+        var stackList = new StackBasedOnList();
+        calculator = new StackCalculator(stackList);
+    }
+    Console.WriteLine("enter the expression to calculate:\n");
+    var expression = Console.ReadLine();
+    var result = (expression != null) ? calculator.Calculate(expression) : new Tuple<bool, double>(false, 0);
+    if (result.Item1 == false)
+    {
+        Console.WriteLine("An expression is incorrect");
+        return;
+    }
+    Console.WriteLine("A result is {0}", result.Item2);


Сместите все, пожалуйста, на 4 пробела влево

viktoriia-fomina · 2023-03-12T08:11:53Z

+        {
+            return new Tuple<bool, double>(false, 0);
+        }
+        var parsedOperands = expression.Split(' ');


Тут тоже можно просто Split()

viktoriia-fomina · 2023-03-12T08:13:11Z

+                        stack.Add(product);
+                        break;
+                    case "/":
+                        if (Math.Abs(stack[size - 1] - 0.0) < Delta)


Можно просто Math.Abs(stack[size - 1]) < Delta

viktoriia-fomina · 2023-03-12T08:18:41Z

+{
+    private static readonly double Delta = 0.00001;
+    private static readonly string[] TestCasesTrue = new[] { "12 8 - 3 *", "-4 9 + 8 * 4 /" };
+    private static readonly double[] CorrectAnswers = new[] { 12.0, 10.0 };


Опять же, все поля нужно именовать со строчной буквы

viktoriia-fomina · 2023-03-12T08:19:39Z

+
+public class StackBasedOnArray : IStack
+{
+    private static readonly double Delta = 0.00001;


А поля с маленькой буквы нужно именовать

viktoriia-fomina · 2023-03-22T19:05:28Z

+
+    public double? Pop();
+
+    public int Size();


Уже гораздо лучше, но все же стандартный интерфейс стека не предполагает того, что стек свой размер знает :)
У стека в его классическом понимании есть указатель на top. На этом факте строятся все операции по работе со стеком. Поэтому вы вполне можете добавить операцию IsEmpty по проверке стека на пустоту и Peek, чтобы "подсмотреть" значение вершины, не извлекая ее. Если конечно вам эти операции могут быть полезны. И на этом в общем-то все

viktoriia-fomina · 2023-03-22T19:09:48Z

+{
+    public void Push(double value);
+
+    public double? Pop();


null возвращать не очень хорошо. Правильным поведением было бы выкинуть исключение в случае если пользователь пытается сделать Pop на пустом стеке. И в следующих домашках от вас это и будет ожидаться :)
Но так как выполняя эту домашку вы не обязаны были знать про исключения, то окей, пусть будет так

viktoriia-fomina · 2023-03-22T19:25:18Z

+    public int Size() => stack.GetLength(0);
+


Опять-таки, метод Size стек не должен предоставлять пользователю.

У вас массив одномерный, можно было просто Length вместо GetLength(0)

viktoriia-fomina · 2023-03-22T19:39:05Z

+        public StackElement(double value)
+        {
+            this.next = null;
+            this.value = value;
+        }
+
+        public StackElement(double value, StackElement? head)
+        {
+            this.next = head;
+            this.value = value;
+        }
+
+        public double Value
+        {
+            get => value;
+            set => this.value = value;
+        }
+
+        public StackElement? Next
+        {
+            get => next;
+        }
+    }


Кажется, что можно обойтись без этого класса :) Он вам нужен исключительно для того, чтобы знать про next элемент.
В массиве у вас элементы лежат подряд, и вы можете получить информацию о том, кто следующий за top. Так что давайте выполнять операции над стеком на массиве из double-ов

viktoriia-fomina · 2023-03-22T19:41:18Z

+    private class StackElement
+    {
+        private double value;
+
+        private StackElement? next;
+
+        public StackElement(double value)
+        {
+            this.next = null;
+            this.value = value;
+        }
+
+        public StackElement(double value, StackElement? head)
+        {
+            this.next = head;
+            this.value = value;
+        }
+
+        public double Value
+        {
+            get => value;
+        }
+
+        public StackElement? Next
+        {
+            get => next;
+        }
+    }


Этот класс не нужен :)
Операции над стеком могут выражаться через операции над List. Например, Push(double element) в стек - это List.Add(double element). List.Add(double element) как раз-таки в конец списка элемент добавляет.
К тому же этот класс - копипаста :) Давайте как-то без копипасты

viktoriia-fomina · 2023-03-22T20:24:07Z

+    {
+        if (expression.Length == 0)
+        {
+            return new Tuple<bool, double?>(false, null);


Здесь стоило бросить исключение, потому что пустой expression - это то, над чем мы не можем выполнить операцию Calculate. Но пусть будет так, потому что, опять же, выполняя эту домашку вы могли еще не знать про исключения

viktoriia-fomina · 2023-03-22T20:24:41Z

+        _stack.Clear();
+    }
+
+    public Tuple<bool, double?> Calculate(string expression)


А сюда могут null передать. Нужна проверка на то, что переданное значение не null

viktoriia-fomina · 2023-03-22T20:30:23Z

@@ -0,0 +1,44 @@
+namespace HW2_Calculator;
+
+public static class Test


Не хватает еще тестов на реализации стеков, но это к следующей домашке относится, так что ок

viktoriia-fomina · 2023-03-22T20:31:24Z

+        if (!Test.TestStackImplementation())
+        {
+            Console.WriteLine("Test has been failed");
+            return;
+        }
+        Console.WriteLine("To choose a stack based on array press 1\nTo choose a stack based on list press 2\n");
+        var isCorrect = Int32.TryParse(Console.ReadLine(), out var option);
+        if (!isCorrect)
+        {
+            Console.WriteLine("Not a number");
+            return;
+        }
+
+        if (option > 2 || option < 1)
+        {
+            Console.WriteLine("Not an option");
+            return;
+        }
+        StackCalculator calculator;
+        if (option == 1)
+        {
+            var stackArray = new StackBasedOnArray();
+            calculator = new StackCalculator(stackArray);
+        }
+        else
+        {
+            var stackList = new StackBasedOnList();
+            calculator = new StackCalculator(stackList);
+        }
+        Console.WriteLine("enter the expression to calculate:\n");
+        var expression = Console.ReadLine();
+        var result = (expression != null) ? calculator.Calculate(expression) : new Tuple<bool, double?>(false, 0);
+        if (result.Item1 == false)
+        {
+            Console.WriteLine("An expression is incorrect");
+            return;
+        }
+        Console.WriteLine("A result is {0}", result.Item2);


Надо все на 4 пробела влево сместить

viktoriia-fomina · 2023-03-22T20:34:35Z

+            {
+                return false;
+            }
+            if (Math.Abs((double)result.Item2 - CorrectAnswers[i]) > delta)


Обращайте внимание на warnings. В домашках их быть не должно :)
Поправьте, пожалуйста

AnNyiiik added 4 commits February 20, 2023 08:36

changed

730c861

created interface IStack, created two classes which implement it, cre…

8952edc

…ated class which ran method calculate (calculate an postfix expression's value), added test class

added tests and input output

779d37b

fixed mistakes

4b5283e

viktoriia-fomina reviewed Mar 12, 2023

View reviewed changes

changed interface IStack, fixed mistakes

ac46d7b

viktoriia-fomina reviewed Mar 22, 2023

View reviewed changes

AnNyiiik added 3 commits March 25, 2023 19:32

fixed

b584f1f

changed

d405ed8

changed

4cd4b28

		@@ -0,0 +1,44 @@
		namespace HW2_Calculator;

		public static class Test

Conversation

AnNyiiik commented Mar 9, 2023

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!

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!

viktoriia-fomina Mar 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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

viktoriia-fomina Mar 22, 2023 •

edited

Loading