---
# System prepended metadata

title: Заметки по ревью кода Гайденко В.

---

# Заметки по ревью кода Гайденко В.

*(тестовое задание в контент WB)*

Ревьюер: [Гульков И.С.](https://t.me/isgulkov)

Версия репы: [`ec6475e48de3131851b8f289b4271bf71ecdc44b`](https://github.com/vladjong/weather_api/tree/ec6475e48de3131851b8f289b4271bf71ecdc44b)

Допы:

- [ ] 1\. Асинхронное обновление данных
- [ ] 2\. Распараллеливание процесса
- [x] 3\. Docker
- [ ] 4\. «Для самых смелых»
- [x] 5\. Swagger, тесты, логи

## Несоответствия требованиям

*Не обнаружены*

## Замечания

*Эти пункты в той или иной мере повлияли на оценку*

### Поддерживаемость и кодстайл

1. Может быть, я не шарю в архитектуре промышленного ПО™️, но всяких пакетов просто *много*, называются они замысловато и выглядит это как абстракция ради абстракции. Зачем нужен интерфейс, если у него только одна реализация? *(чтобы в IDE команда "Go to definition" не работала? 🌝)* Что значит "use case" в названии `weatherServiceUseCase`?

   Наиболее концентрированный пример проблем с читаемостью этого кода — вот эта сборка матрешки на ровном месте:

   ```go
   openWeatherApi := openweather.NewOpenWeatherApi(s.cfg.WeatherAPI.Limit, os.Getenv("API_KEY"), s.cfg.WeatherAPI.Units)
   postgres := postgressql.NewWeatherServiceStorage(s.postgresClient)
   useCase := usercase.NewWeatherServiceUseCase(postgres, openWeatherApi)
   weatherService := externalservice.NewOpenWeatherApi(useCase)
   ```
   
   Какая бы тут ни была красивая архитектурная идея, она не считывается; зато создается полное ощущение, что над этим кодом работают уже 10 лет и мечтают переписать с нуля.

### Работа с внешним API

1. Методы похода по HTTP не возвращают ошибку, зато на любой ошибке крэшат сервер с "фатал еррором". То, что какой-то внешний сервис не ответил или прислал HTML вместо JSON — это крайне вероятная ошибка, которую можно и нужно обрабатывать, помирать здесь недопустимо

## Придирки по-мелочи

*Эти пункты __не__ повлияли на оценку*

### Работа с БД

- Конфигурируемые названия таблиц, которые потом вставляются с помощью `fmt.Sprintf` — это че-то какая-то дичь...
- Для GPS-координат (`lat`+`lon`) не нужно и не следует использовать десятичный тип (`numeric` = `decimal`) — это как раз тот случай, когда `double` подходит идеально

