Skip to content

Спринт 2 задание by Алексей Паничев - #1

Open
AlexPanic wants to merge 4 commits into
mainfrom
dev
Open

AlexPanic wants to merge 4 commits into
mainfrom
dev

Conversation

@AlexPanic

@AlexPanic AlexPanic commented Jun 28, 2023 •

Copy link
Copy Markdown
Owner

попытка 2

Comment thread src/main/java/Calc.java Outdated
}
this.addItem(item);
return 1;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Лучшей практикой будет выносить такие коды (1, -1, 0) в константы при помощи final. Так будет понятно из названия константы, что она обозначает, можно будет отслеживать, где данная константа ещё используется, просто кликнув ctrl+click по объявленной константе. А то сейчас сходу не понятно, что означают эти числовые коды
Плюс если кодовое число используется не в одном месте, как сейчас, то не будет риска поменять в одном месте и забыть в другом

Comment thread src/main/java/Calc.java Outdated
}
}

String getWordForm(double sum, String f1, String f2, String f5) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  1. Если планируем использовать только в данном классе, стоит сделать private
  2. Лучше давать аргументам более очевидные названия. Из названия f1, f2, f3 непонятно, что они означают

Comment thread src/main/java/Main.java Outdated
} else {
people = peopleIn;
return true;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Не совсем понятно, почему вся логика по обработка ввода, выводу результата, склонении окончаний находится в одном общем класса, а логика по обработке ввода людей - в Main. Некритично, но можно также вынести

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.

2 participants