IliasNumberOne - CurrencyExchangeApi.md

https://github.com/IliasNumberOne/CurrencyExchangeApi

Ревью от Евгения @zhekadoe

Пишу в первую очередь для себя, на истину не претендую

CurrencyDao

create

var keys = statement.getGeneratedKeys();
if(keys.next()) {
  currencyModel.setId(keys.getInt(1));
}
return currencyModel;

if не нужен. Если не сможет вставить, выкинет исключение. Дополнительно в запросе можно использовать RETURNING

PSQLException exception = (PSQLException) e;
if(exception.getSQLState().equals("23505")) {
Магическое число, поработав с отладчиком видно что кидается потомок у которого есть своя константа
} catch (SQLException e) {
  if (e instanceof SQLiteException sqliteE &&
      sqliteE.getResultCode() == SQLITE_CONSTRAINT_UNIQUE) {

findAll findByCode

final String FIND_ALL_SQL= """
  SELECT * FROM currencies
  """;

Вынесение константы в глобальные поможет переиспользовать ее

prsf St FIND_BY_CODE_SQL = FIND_ALL_SQL + """
  WHERE code = ?
  """

getInstance

Находиться после приватного метода, мелочь конечно.

update delete

Дополнительный функционал хорошо, плохо что от вставлен между findAll и findByCode и нигде далее не используется. Можно реализовать методы PATCH & DELETE для Currency

ExchangeRatesDao

create update

Тут скорее имхо. Передача ExchangeRate в качестве параметра приводит к усложнение логики на стороне сервиса, а при его отсутствии сервлета. Поиск валюты по его коду это зона ответственности именно уровня Дао. Передача кодов валют позволит выделить общие части в этих двух методах

findByCrossConvert

Очень удачное решение, его логичным продолжением будет findDirectRate (бывшее findByCode) и findReverseRate. Слова "By" & "Convert" излишне. Еще можно подумать над передачей параметра

select er1.id as id,
     baseCr1.id as base_currency_id,
     baseCr2.id as target_currency_id,
     round(?) as rate

interface Dao<K, T>

findById нигде не используется. По факту тут ключами являются код валюты для Currency и пара кодов валюты для ExchangeRate и в легаси проектах скорей всего так и будет. "id serial primary key" предназначен для удобства работы с ORM фреймворками. Если хочется использовать интерфейс то желательно передавать некое ДТО

ExchangeCurrencyDto, etс.

ДТО inmutteble поэтому вместо "@Data" лучше использовать "@Value" и сразу задавать ему статический конструктор '@Value(staticConstructor = "of")'

convertedAmount можно вычислять геттером

public class ExchangeCurrencyDto {
  //..
    private BigDecimal convertedAmount;
}

//
public class ExchangeCurrencyDto {
    //..
    
    public BigDecimal getConvertedAmount() {
        return amount.multiply(rate);
    }
}

application.properties

Для удобства деплоя можно подправить файл "/etc/hosts " или "c:\windows\system32\drivers\etc\hosts" в linux или windows соответственно 194.87.102.96 myhost

Легко можно добавить авторизацию на сервере без ввода пароля и скопировать файл настроек прямо в приложение на сервер

ssh-keygen
ssh-copy-id username@myhost
scp production/application.property username@myhost:/path/to/app/WEB-INF/