Full project review by Alexander Khmelov#1
Conversation
There was a problem hiding this comment.
Фух. Много. Под конец уже внимания нет.
Код интересный, мидлее стал. Но надо порядок в слоях наводить.
И однотипнее все пилить.
- Представь что у тебя 500 сервисов и 500 контроллеров для web.
- Представь что просят десктопную версию.
- А потом консольную.
Что делать? Вот так должен быть сделан код.
Ну меня радует. Рост хороший. Security надо. И архитектуру. Все иное ок.
| @Component | ||
| public class GlobalExceptionAspect { | ||
|
|
||
| @AfterThrowing(pointcut = "execution(* com.javarush.quest.shubchynskyi..*(..))", throwing = "ex") |
There was a problem hiding this comment.
Насколько я понимаю это логирование уровня приложения. Довольно часто логи распределяют либо по слоям, либо по признакам домена предметной области. Можно что-то модифицировать в этом направлении.
| import java.util.Locale; | ||
|
|
||
| @RequiredArgsConstructor | ||
| public class AppLocaleResolver implements LocaleResolver { |
There was a problem hiding this comment.
Тут по-моему все нормально. Не добавить не убавить, классика.
|
|
||
| @Bean | ||
| public LocaleChangeInterceptor localeChangeInterceptor() { | ||
| LocaleChangeInterceptor lci = new LocaleChangeInterceptor(); |
There was a problem hiding this comment.
Имена переменных я бы не сокращал нигде
| private String migrationName; | ||
|
|
||
| @Override | ||
| public void run(String... args) { |
There was a problem hiding this comment.
Тут не очень понятно какой будет объем но если он небольшой можно подумать о транзакциях. Будет быстрее.
| public void run(String... args) { | ||
|
|
||
| try { | ||
| Integer count = jdbcTemplate.queryForObject( |
There was a problem hiding this comment.
А хотя нет я ошибся. Думал что сделано на orm а тут jdbc
|
|
||
| @Slf4j | ||
| @Service | ||
| public class ImageService { |
| @Slf4j | ||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class QuestService { |
There was a problem hiding this comment.
Это пополам. Часть тут а часть в контроллере. ОДНОТИПНЕЕ надо.
|
|
||
| @Transactional | ||
| public void create(Question question) { | ||
| questionRepository.save(question); |
|
|
||
| public UserDataProcessResult processUserData( | ||
| UserDTO userDTOFromModel, | ||
| BindingResult bindingResult, |
There was a problem hiding this comment.
А тут похоже на проваливание абстракций в сервис. Он будет работать без web?
| return updatedUser; | ||
| } | ||
|
|
||
| public UserDataProcessResult processUserData( |
2 participants
No description provided.