Hw2 calculator #3
Conversation
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
По существу всё правильно, но уж очень неаккуратно
| if ((!isCorrect1 || !isCorrect2) || (result1 != mainResult || result2 != mainResult)) | ||
| { | ||
| return false; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
Какой ужас :) Просто return то, что написано в if
|
|
||
| private bool CheckFalseExpression(bool isCorrect1, bool isCorrect2) | ||
| { | ||
| return (!isCorrect1 && !isCorrect2); |
There was a problem hiding this comment.
Тут, кстати, скобки лишние :)
| return (!isCorrect1 && !isCorrect2); | ||
| } | ||
|
|
||
| [TestCase] |
There was a problem hiding this comment.
На самом деле, просто Test, TestCase --- это если бы Вы параметры в метод передавали
| { | ||
| private bool CheckTrueExpression(bool isCorrect1, bool isCorrect2, double result1, double result2, double mainResult) | ||
| { | ||
| if ((!isCorrect1 || !isCorrect2) || (result1 != mainResult || result2 != mainResult)) |
There was a problem hiding this comment.
Оо, кто-то всё-таки сравнивает double-ы через !=. Это не совсем == и сравнение с нулём, но уровень непонимания представления вещественных чисел такой же :) См. https://0.30000000000000004.com/
| IStack stack2 = new StackArray(); | ||
| stack1.Push(9); | ||
| stack2.Push(8); | ||
| Assert.IsTrue((stack1.Pop() == 9 && stack2.Pop() == 8)); |
There was a problem hiding this comment.
Правильнее было бы написать два Assert.AreEqual. Тут, если тест не пройдёт, Вам скажут "ну, тест не прошёл", а если сделать правильно, то напишут, что конкретно чему не равно, что упростит отладку
| private static bool IsOperand(char symbol) | ||
| { | ||
| return symbol == '+' || symbol == '-' || symbol == '*' || symbol == '/'; | ||
| } |
There was a problem hiding this comment.
Можно было через =>
| @@ -0,0 +1,60 @@ | |||
| using NUnit.Framework; | |||
|
|
|||
| namespace hw2Calculator.Test | |||
There was a problem hiding this comment.
Пространства имён в .NET именуются с заглваной всегда
| public bool IsEmpty() | ||
| { | ||
| return countNumbersInStack == 0; | ||
| } |
There was a problem hiding this comment.
Тоже лучше через =>
| return countNumbersInStack == 0; | ||
| } | ||
|
|
||
| public void DeleteStack() |
There was a problem hiding this comment.
Скорее, ClearStack, чтобы не смущать C++-ников и вообще. Мы же стек не удаляем, мы просто выкидываем все его элементы и возвращаем его в начальное состояние
Co-authored-by: Yurii Litvinov <yurii.litvinov@gmail.com>
| IStack stack1 = new StackList(); | ||
| IStack stack2 = new StackArray(); | ||
| (var result1, var isCorrect1) = Calculator.CalculatorExpression(expresion, stack1); | ||
| (var result2, var isCorrect2) = Calculator.CalculatorExpression(expresion, stack2); |
There was a problem hiding this comment.
Условие "при этом не содержать дублирующегося кода" не выполнено. Посмотрите https://docs.nunit.org/articles/nunit/writing-tests/attributes/testcasesource.html и https://docs.nunit.org/articles/nunit/writing-tests/attributes/testcase.html
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Надо ещё немного поправить
| @@ -0,0 +1,60 @@ | |||
| using NUnit.Framework; | |||
|
|
|||
| namespace hw2Calculator.Test | |||
|
|
||
| private bool CheckFalseExpression(bool isCorrect1, bool isCorrect2) | ||
| { | ||
| return (!isCorrect1 && !isCorrect2); |
There was a problem hiding this comment.
Тут, кстати, скобки лишние :)
| { | ||
| private bool CheckTrueExpression(bool isCorrect1, bool isCorrect2, double result1, double result2, double mainResult) | ||
| { | ||
| return (!isCorrect1 || !isCorrect2) || (result1 - mainResult < 0.000001 || result2 - mainResult < 0.000001); |
There was a problem hiding this comment.
Ой, математика говорит, что если mainResult достаточно большой, эта штука его одобрит
| return (0, false); | ||
| } | ||
|
|
||
| private static bool IsOperand(char symbol) |
| private static bool IsOperand(char symbol) | ||
| { | ||
| return symbol == '+' || symbol == '-' || symbol == '*' || symbol == '/'; | ||
| } |
| return !isCorrect || Math.Abs(result - mainResult) < 0.000001; | ||
| } | ||
|
|
||
| private static (string Test, double Result)[] CorrectDataForTests = new[] |
There was a problem hiding this comment.
Вообще, это поле, так что должно быть со строчной по идее
| new TestCaseData(new StackArray(), DataForTests.Test, DataForTests.Result) | ||
| }); | ||
|
|
||
| private static string[] UncorrectDataForTests = new[] |
There was a problem hiding this comment.
"IncorrectDataForTests"
| [TestCaseSource(nameof(CorrectDataForTest))] | ||
| public void CalculatorTestForUncorrectData(IStack stack, string expresion, double mainResult) |
There was a problem hiding this comment.
Что-то имя источника данных в TestCaseSource противоречит имени теста
| Assert.IsTrue(CheckTrueExpression(isCorrect, result, mainResult)); | ||
| } | ||
|
|
||
| private static IEnumerable<TestCaseData> UncorrectDataForTest |
There was a problem hiding this comment.
Ещё, кстати, у Вас две пары полей, которые называются почти одинаково. Это плохо, это запутывает читателя. Назовите их по смыслу.
| IStack stack1; | ||
| IStack stack2; |
| stack1 = new StackList(); | ||
| stack2 = new StackArray(); |
There was a problem hiding this comment.
Тут тоже стоило data-driven-тесты использовать, потому что стеки реализуют один интерфейс и обязаны вести себя одинаково
hw2Calculator/hw2Calculator/Test.cs
Outdated
|
|
||
| namespace Hw2Calculator | ||
| { | ||
| class Test |
There was a problem hiding this comment.
А это теперь не надо :)
| double delete = stackElements[countNumbersInStack]; | ||
| return delete; |
There was a problem hiding this comment.
Но зачем? Это же просто return stackElements[countNumbersInStack];
| public bool IsEmpty() | ||
| => countNumbersInStack == 0; | ||
|
|
||
|
|
There was a problem hiding this comment.
Лишняя пустая строчка
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Да, теперь всё хорошо, зачтена
| } | ||
|
|
||
| private static IEnumerable<TestCaseData> UncorrectDataForTest | ||
| private static IEnumerable<TestCaseData> TestIncorrectDate |
There was a problem hiding this comment.
Date --- в смысле "свидание"?
| private bool CheckTrueExpression(bool isCorrect, double result, double mainResult) | ||
| { | ||
| return !isCorrect || Math.Abs(result - mainResult) < 0.000001; | ||
| } |
3 participants
No description provided.