Skip to content

implemented task lambda calc#304

Open
alex-hazniuk wants to merge 2 commits into
mate-academy:masterfrom
alex-hazniuk:hw-lambda-calc
Open

implemented task lambda calc#304
alex-hazniuk wants to merge 2 commits into
mate-academy:masterfrom
alex-hazniuk:hw-lambda-calc

Conversation

@alex-hazniuk
Copy link
Copy Markdown

No description provided.

class CalculatorTest {
private static final double DELTA = 0.1;
private static Calculation count;
private double actual;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

цю змінну нема сенсу виносити на рівень класу, вона для усіх методів повинна відрізнятись

@Test
void calculate_illegalOperation_isNotOk() {
assertThrows(IllegalOperationException.class, () -> {
count.calculate(0, 2, '%');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
count.calculate(0, 2, '%');
count.calculate(0, 2, invalidOperation);

для читаймості краще цей чар оголосити змінною в методі


public class Calculator implements Calculation {
@Override
public double calculate(double number1, double number2, char operation) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

плохая практика использовать числа в названии переменных и аргументов

case '^':
return Math.pow(number1, number2);
default:
throw new IllegalOperationException("Illegal operator");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
throw new IllegalOperationException("Illegal operator");
throw new IllegalOperationException("Illegal operator " + operation);

лучше делать сообщение ошибок как можно более информативными

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