Iter8 #8
Iter8 #8
Conversation
cmd/shortener/main.go
Outdated
| "shortURL/internal/handlers" | ||
| ) | ||
|
|
||
| func main() { | ||
| r := handlers.NewRouter() | ||
| Params := app.GetEnv() |
There was a problem hiding this comment.
а зачем с большой буквы имя переменной?
There was a problem hiding this comment.
И если это конструктор (возвращает экземпляр структуры), то нужно именовать New<имя_создаваемой_сущности), подробнее тут
- также, как
NewRouter()и также с ним работать
internal/app/DBmethods.go
Outdated
| @@ -0,0 +1,82 @@ | |||
| package app | |||
There was a problem hiding this comment.
Имя файла вдруг в CamelCase, хотя остальные snake_case (обычно именно его используют)
И DB методы в пакете app? Так не пойдёт, надо отдельный пакет создавать, а то "каша" будет
There was a problem hiding this comment.
И вообще для хранилки - фабрика нужна. Скинул в группу ссылку с примером
cmd/shortener/main.go
Outdated
| "shortURL/internal/handlers" | ||
| ) | ||
|
|
||
| func main() { | ||
| r := handlers.NewRouter() | ||
| Params := app.GetEnv() | ||
| Params.OpenDB() |
There was a problem hiding this comment.
OpenDB в пакете app точно не место. EVN и flags ещё можно оставить (но их по-хорошему бы в независимый пакет для конфига). Но DB - это точно отдельный контроллер (controller / facade / repository и тп паттерны)
internal/app/DBmethods.go
Outdated
| BaseURL = make(map[string]string) | ||
| ) | ||
|
|
||
| type DBstring struct { |
There was a problem hiding this comment.
Имя переменной должно точно отражать, о функциональности переменной, а не реализации. Тоесть: dbSlice, dbStruct, dataMap и тому подобное - плохой нейминг. Нужно что-то, что отражает именно эту переменную - ShortenUrls, URLList или что-то подобное.
internal/app/DBmethods.go
Outdated
| decoder *json.Decoder | ||
| } | ||
|
|
||
| func NewReaderDB(P *Param) (*readerDB, error) { |
There was a problem hiding this comment.
У нас, всё же, не база данных, а файловый reader/writer
internal/app/DBmethods.go
Outdated
| encoder *json.Encoder | ||
| } | ||
|
|
||
| func NewWriterDB(P *Param) (*writerDB, error) { |
There was a problem hiding this comment.
Всё же отдельный пакет нужен, Storage, иначе что-то "страшное" получается. Надо использовать паттерн Фабрика, только интерфейс назвать Storager
internal/app/envinronment.go
Outdated
| "github.com/caarlos0/env/v6" | ||
| ) | ||
|
|
||
| type Param struct { |
There was a problem hiding this comment.
Это почти везде - Config
There was a problem hiding this comment.
Супер всё, молодец, прям разобрался в коментах, постарался, молоток)
Пару моментов с неймингом, но это вообще мелочи: по имени конфига предложение, по конструктору стораджа точно лучше NewStorage (а не NewStorager), так как в итоге всё же структуру вернём, который удовлетворяет интерфейсу.
Всё круто, апрув, в след спринте исправить 5 сек
| ) | ||
|
|
||
| func main() { | ||
| r := handlers.NewRouter() | ||
| params := config.NewEnv() |
There was a problem hiding this comment.
Config обычно всегда config. Приблизительно так будет читабельней:
c := config.NewConfig()
Инкремент №8