-
Notifications
You must be signed in to change notification settings - Fork 156
Тестовое задание для Райффайзенбанк #173
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
Файл README.md содержит описание работы с приложением
| <dependency> | ||
| <groupId>io.rest-assured</groupId> | ||
| <artifactId>rest-assured</artifactId> | ||
| <version>4.4.0</version> |
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.
все версии нужно выносить в секцию properties
| import org.springframework.data.jpa.repository.config.EnableJpaRepositories; | ||
|
|
||
| @SpringBootApplication | ||
| @EnableJpaRepositories("ru.raiffeisen.soksapp.repositary") |
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.
ошибка в слове repository
|
|
||
| private static final Logger log = LoggerFactory.getLogger(SocksController.class); | ||
|
|
||
| @Autowired |
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.
@Autowired на поле - плохая практика, тк отсутствует возможность внедрить сервис с ожидаемым поведением
|
|
||
| try { | ||
| sockService.deleteSocks(sock); | ||
| } catch (NotEnoughSocsException e) { |
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.
обработка исключений в контроллере - плохая практика, для этого нужно создавать отдельный класс, который перехватывает и обрабатывает исключения приложения
| log.error("Произошла ошибка при удалении носков", e); | ||
| return new ResponseEntity<>("Количество носков на складе меньше, чем в запросе", HttpStatus.BAD_REQUEST); | ||
| } | ||
| return ResponseEntity.ok().build(); |
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.
также если обрабатывать исключения в другом классе можно пометить метод как void и навесить аннотацию @ResponseStatus
|
|
||
| if (newQuantity == 0) { | ||
| log.info("Количество носков стало равным 0, удяляем запись с id: " + sockEntity.getId()); | ||
| repository.deleteById(sockEntity.getId()); |
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.
кажется излишним действием, ведь мы во время прихода носков всё равно лезем в базу и проверяем есть ли у нас такие
| .sum(); | ||
| } | ||
|
|
||
| throw new RuntimeException("Unsupported operation is passed: " + operation); |
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.
это должно быть внутри switch default case
| .mapToInt(SockEntity::getQuantity) | ||
| .sum(); | ||
|
|
||
| case MORE_THAN: // че-то |
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.
комментарии в коде должны быть на английском, если они и есть вообще
| .post("/api/socks/income") | ||
| .getStatusCode(); | ||
|
|
||
| Assertions.assertThat(statusCode) |
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.
к верхнему действию можно добавить .andExpect...
|
|
||
| List<SockEntity> allSocks = socksRepository.findAll(); | ||
|
|
||
| int newStatus = RestAssured.given() |
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.
два теста в одном тесте - плохая практика
Файл README.md содержит описание работы с приложением