Замечания по shortlink

[x] бинарник нет смысла коммитить в репозиторий(также как и файлы логов, сторонних библиотек, настроек ide и тп)
[x] перед разыменованиемif *file {, нужно убедится, что указатель не nil, иначе будет беда
[x] domain это из области DDD, тут просто обычная модель, её можно сразу хранить по месту требования, или пакет назвать model
[x] странное название для модели Json, лучше подойдут URLMapping, ShortLinks и тп
[x] можно логировать сразу все ошибки снаружи вызова InitService, сейчас очень много бесполезного логирования
[x] предлагаю сделать следующим образом, сервер стартуем в main, там же объявляем handler и связываем их, т.к. сейчас получился оверхэд с глобальными переменными
[x] нужно объявить структуру(к примеру server), которая будут хранить logger, handler, paths и другие глобальные сущности, чтобы не надеятся, что эти глобальные переменные кто-то проинициализировал
[x] странно прописана логика редиректа, можно просто получить урл и прописать его в заголовок, не опираясь на js-костыли см Header Location(https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Location), главное не забыть выставить запросу статус redirect 302
[x] видимо хотел попробовать разделение на слоистую архитектуру. Сейчас получилось так, что app-слой, который должен отвечать за логику приложения, отвечает за инфраструктуру(как это разворачивается, что нужно залогировать,, какие ошибки выдать), должно быть наоборот - http-сервер вызывает приложение, как некий сервис тут во второй части лекции есть про чистую архитектуру, но думаю, пока изучать её рановато https://docs.google.com/presentation/d/e/2PACX-1vTH1nPDE4aDIa5bpTg_EfeNzo7xz5cZF73R5xbmDqTIBPikBCDvaYBjUTu4djPtwzwMIpiYkM7mqtUx/pub?start=false&loop=false&delayms=3000&slide=id.gc50bcc1b87_0_0

[ ] опять же, глобальные переменные в main - непонятно зачем, их можно объявлять прямо внутри функции, хранить их можно в структуре httpHandler, httpServer и тд. Глобальные переменные это скорее антипаттерн, т.к. каждый раз при взаимодействии с ними, тебе приходится думать о том, кто и как эти переменные использует и проинициализированы ли они.
[ ] getKillSignalChan названа как будто она что-то возвращает, на деле инициализирует поле httpServer, лучше её сразу вызывать в методе startServer, а ещё лучше прямо в функции-констркуторе httpServer
[ ] logger.LogServerError(URL, "successful") это и не ошибка, и в целом бесполезно, т.к. у пользовательская операция будет выполнена
[ ] в случае, когда ссылка не найдена - нужно отвечать 404 и опять же можно не логировать. А вот внутренняя ошибка это 500, и лог нужен
[ ] отдельно обрабатывать "/" не надо - подойдёт общее поведение с 404, если пользователь не указал её в маппинге
[ ] тут наглядно видно, как неудобно использовать паттерн с глобальной ошибкой 1) мы не можем понять в какой момент исполнения программы произошла ошибка или придётся постоянно дергать GetErr 2) А что если будет 2 одновременных запроса? оба упадут из-за ошибки в одном 3) После обработки запроса - сервис в невалидном состоянии, работать больше не может. Для одноразовой утилиты это норма, но в нашем случае мы имеем дело с http-сервером, который крутится постоянно, будет плохо если из-за одной неправильной ссылки мы упадём

Что же делать: Очень просто, в каждом методе, где может быть ошибка - возвращать её, а не менять глобальное состояние объекта

[ ] JsonProvider - это скорее PathStorage, у него тоже как у сервиса можно завести метод Create, где сразу вызывать LoadPaths
[ ] сейчас app пакет зависит от infrastructure import ( "golang-shortlink/pkg/shortLinkService/infrastructure/jsonProvider" ) Чтобы это поправить, можно просовывать JsonProvider в сервис снаружи из main. Тогда сервису будет доступен интерфейс, а о наюнсах создания, инициализации JsonProvider и тд он знать не будет

veresnikov / golang-shortlink

Замечания по shortlink #1