Implemented calculator#284
Conversation
|
|
||
| @Test | ||
| void addictionTwoNegative() { | ||
| number1 = -7; |
There was a problem hiding this comment.
you use the same numbers everywhere, so you can simplify the code, make them constants
There was a problem hiding this comment.
Часто бачив, як в наших тестах до завдань константи до чисел не записувались, тому вирішив прописати всі числа напряму
Впринципі, якщо ментор зможе мене переконати, що це так важливо, то можна змінити
There was a problem hiding this comment.
не обязательно выносить это в константы, но в тестах лучше использовать разные данные, это повышает вероятность найти ошибку
| package core.basesyntax; | ||
|
|
||
| public class Calculator { | ||
| public double calculate(double number1, double number2, char operation) { |
There was a problem hiding this comment.
It's bad practice to use numbers in variable names
| return Math.pow(number1, number2); | ||
| case '/': | ||
| if (number2 == 0) { | ||
| throw new IllegalArgumentException("Can't divide by zero"); |
There was a problem hiding this comment.
maybe ArithmeticException would be better here?
| } | ||
| return number1 / number2; | ||
| default: | ||
| throw new IllegalStateException("Invalid operation"); |
There was a problem hiding this comment.
Maybe IllegalArgumentException would be better here?
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class CalculatorTest { | ||
| private static final Calculator calculator = new Calculator(); |
There was a problem hiding this comment.
lets initialize this variable in @BeforeAll method
There was a problem hiding this comment.
Why do we need it?
We can just initialize the variables at the beginning
There will be fewer lines of code
There was a problem hiding this comment.
Обычно в тестах объекты сложные и требуют определенной доинициализации (что делается в методе @BeforeAll). В данном случае объект простой, и в принципе можно было оставить инициализацию в момент объявления, но мы хотим что б вы знали про метод @BeforeAll и умели им пользоваться.
| private double number1; | ||
| private double number2; |
|
|
||
| @Test | ||
| void addictionTwoNegative() { | ||
| number1 = -7; |
There was a problem hiding this comment.
не обязательно выносить это в константы, но в тестах лучше использовать разные данные, это повышает вероятность найти ошибку
| } | ||
|
|
||
| @Test | ||
| void addictionZero_1() { |
There was a problem hiding this comment.
It is also bad practice to use numbers in method names.
| @Test | ||
| void addictionZero_1() { | ||
| number1 = 0; | ||
| number2 = -14; | ||
| expected = -14; | ||
| actual = calculator.calculate(number1, number2, '+'); | ||
| assertEquals(expected, actual); | ||
| } | ||
|
|
||
| @Test | ||
| void addictionZero_2() { | ||
| number1 = -14; | ||
| number2 = 0; | ||
| expected = -14; | ||
| actual = calculator.calculate(number1, number2, '+'); | ||
| assertEquals(expected, actual); | ||
| } |
There was a problem hiding this comment.
we can combine this tests, they test the same logic
| number1 = 7; | ||
| number2 = -14; | ||
| expected = -7; | ||
| actual = calculator.calculate(number1, number2, '+'); | ||
| assertEquals(expected, actual); |
There was a problem hiding this comment.
I think it worth to add here the check with first negative operant and second positive
3 participants
No description provided.