-
Notifications
You must be signed in to change notification settings - Fork 0
Выполнил задание, исправил уязвимости #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
vpyzhyanov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
есть замечания
pom.xml
Outdated
| <dependency> | ||
| <groupId>org.junit.jupiter</groupId> | ||
| <artifactId>junit-jupiter</artifactId> | ||
| <scope>test</scope> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scope надо писать в другом месте
pom.xml
Outdated
|
|
||
| <dependency> | ||
| <groupId>io.netty</groupId> | ||
| <artifactId>netty-transport-native-epoll</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вместо того, чтобы определять кучу библиотек одной группы можно немного подумать как оптимизировать процесс, притом сделая код более надёжным. Надо посмотреть а есть ли у этих библиотек общий родитель.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
исправил
pom.xml
Outdated
|
|
||
| <dependency> | ||
| <groupId>io.netty</groupId> | ||
| <artifactId>netty-resolver-dns-native-macos</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вам похоже было совсем не лень перечислять все)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Да, исправил
|
|
||
| String discordToken = System.getenv("discord_token"); | ||
| new DiscordBot(discordToken) | ||
| new DiscordBot(discordToken, new MarkHandler()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не выполнено требование задачи
И для телеграмма и для дискорда должна использоваться одна и та же обработка сообщений
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Исправил
src/main/java/ru/urfu/Bot.java
Outdated
| * @see DiscordBot | ||
| * | ||
| */ | ||
| public interface Bot { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
покажите мне место, где вы используете этот интерфейс
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нигде, убрал
| */ | ||
| public class MarkHandler implements Handler{ | ||
| /** | ||
| * Обработать сообщение от пользователя |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вы нарушаете принцип Барбары Лисков и в целом полиморфизм. Нельзя переопределять описание методов интерфейса. JavaDoc определяется один раз в интерфейсе, остальные методы должны ему строго соответствовать
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Исправил
| } | ||
|
|
||
| /** | ||
| * Запустить Телеграмм бота |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
И тут
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Исправил
src/test/java/MarkHandlerTest.java
Outdated
| */ | ||
| @Test | ||
| void handleReturnsExpectedResponseWithInputMessage() { | ||
| Handler handler = new ru.urfu.MarkHandler(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
замените на нормальный импорт
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Исправил
src/test/java/MarkHandlerTest.java
Outdated
| void handleReturnsExpectedResponseWithInputMessage() { | ||
| Handler handler = new ru.urfu.MarkHandler(); | ||
| String inputMessage = "Hello, Bot!"; | ||
| String expectedResponse = "Ваше сообщение: 'Hello, Bot!'"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
в создании этих переменных не особо много смысла. Зато они раздувают очень краткий код до невероятных пределов
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Исправил
src/main/java/ru/urfu/Handler.java
Outdated
| * <br> | ||
| * Благодаря этому интерфейсу | ||
| * можно реализовать несколько разных обработчиков | ||
| * и использовать их в разных ботах, а также менять обработчики, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
С одной стороны это хорошо, но этим вы нарушаете требования задачи. Нужно было одну логику использовать, а вы позволяете где менять везде где только можно, притом независимо
vpyzhyanov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
принято
No description provided.