-
Notifications
You must be signed in to change notification settings - Fork 156
Решение тестового задания #174
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: main
Are you sure you want to change the base?
Conversation
| } | ||
|
|
||
| @Transactional | ||
| public ResponseEntity<SocksDBModel> registerSocksIncome(SocksRequestData data) { |
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.
Ответ обычно оборачиваем в другой объект, dto, хотя по тз и так допустимо, но в сложных больших приложениях почти никогда в ответе не возвращаем объект как он лежит в бд
| public class SocksUtil { | ||
|
|
||
| //null if data is not valid | ||
| public static SocksDBModel getSockFromRequestData(SocksRequestData data) { |
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.
чем плоха статика - что мы не можем легко подменить реализацию - нам надо будет каждый класс, где используется SocksUtil изменить. Лучше использовать спринговый бин для такого.
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.
Вот за этот комментарий огромное спасибо, тк во время написания кода возникал вопрос, как сделать правильно.
| case MORE_THAN: | ||
| result = repository.getQuantityByColorAndCottonPartMoreThan(color, cottonPart); | ||
| break; | ||
| default: |
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.
тоже наверное badRequest было бы лучше - понятнее что ли
| if (dbModelData == null | ||
| || getById(dbModelData.getId()) == null | ||
| || countQuantityAfterOutcome(dbModelData) < 0) { | ||
| return new ResponseEntity<>(HttpStatus.BAD_REQUEST); |
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.
вот кстати в badRequest хорошо бы возвращать, что в запросе плохо, то есть описание
| } | ||
| SocksDBModel dbSocks = getById(dbModelData.getId()); | ||
|
|
||
| long quantityAfterSubtraction = dbSocks.getQuantity() - data.getQuantity(); |
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.
отрицательное число получается тоже может быть?)
| @@ -0,0 +1,22 @@ | |||
| package ru.simbial.cibinternstesttask.app.model; | |||
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.
Резюмируя - задача выполнена хорошо, я считаю. Какие-то недочеты есть, комменты оставил. Их не стоит воспринимать на личный счет- точно такие же ошибки совершают разработчики, в том числе и я - ежедневно. Некоторые моменты вообще спорные, поэтому все ок
Инструкции находятся в файле INSTRUCTIONS.MD