aleos-dev - currency-converter.md

https://github.com/aleos-dev/currency-converter
Ревью от Ивана
Ревью на 3 проект от @HTSWT В целом имплементация мне понравилось, видно что автор постарался. Ну а теперь пойду закрою форточку.

Deploy

1. Ты завернул томкат в контейнер плюс тебя уже есть release-actions, но все еще есть место для доработок. Можно добавить еще пару шагов c билдом образа и пушем его в докерхаб, например и таким образом можно упростить развертывание приложения на сервере (не нужно будет клонировать все сурсы)

2. *Можно пойти дальше и посмотреть в сторону watchtower (он будет наблюдать за обновлениями образа, пуллить свежие версии и рестартить контейнеры)

Application

1. Парсингом запроса все-таки должен заниматься контроллер, а не сервис, поэтому я бы явно сделал два метода ::findByCode и findById. Для сервиса code - это строка в 3 символа, все остальное должно было предварительно провалидировано. И тут явно напрашивается использование естественного ключа для currency. Этим можно избавиться от путанницы с двумя идентификаторами, а еще чуть увеличить перформанс sql-запросов, где ты джоинишь по code, а не по id (кластерный индекс все-таки) https://github.com/aleos-dev/currency-converter/blob/9b3a7a4e601acb0995a0d6b9ed3e4d10a519d864/backend/src/main/java/com/aleos/service/CurrencyService.java#L36

2. Я бы вынес преобразование из BigDecimal в Double в какой-нить класс, чтобы не повторялась логика по скейлингу и округлению https://github.com/aleos-dev/currency-converter/blob/9b3a7a4e601acb0995a0d6b9ed3e4d10a519d864/backend/src/main/java/com/aleos/service/ConversionService.java#L36

3. Я бы не стал отдавать наружу апи для кэширования. Кэширование - это логика хранения данных и должна находится на уровне dao. Типичное кэширование делают через декоратор, оборачивают основной репозиторий (в спринге - через аннотации). Тогда можно будет избавиться от сложного CachingFilter https://github.com/aleos-dev/currency-converter/blob/9b3a7a4e601acb0995a0d6b9ed3e4d10a519d864/backend/src/main/java/com/aleos/service/CacheService.java#L7

4. Спорное решение делать тут абстрактный класс CrudDao, на мой взгляд получилось сильно сложнее, чем если бы ты забил на соблюдения dry тут. Возьмем к примеру метод save - ты заранее предполагаешь, что в переданном запросе результатом будет сгенерированный id, но это не интуитивно и не следует из сигнатуры метода. https://github.com/aleos-dev/currency-converter/blob/9b3a7a4e601acb0995a0d6b9ed3e4d10a519d864/backend/src/main/java/com/aleos/dao/CrudDao.java#L120

5. В sql есть ключевое слово RETURNING, где можно указать результат выполнения запроса (например, после инсерта можно вернуть запись целиком) https://github.com/aleos-dev/currency-converter/blob/9b3a7a4e601acb0995a0d6b9ed3e4d10a519d864/backend/src/main/java/com/aleos/dao/ConversionRateDao.java#L297

Думаю, что можно потихоньку смотреть в сторону спринга, в сервлетах вроде уже достаточно хорошо разбираешься