-
Notifications
You must be signed in to change notification settings - Fork 0
Increment13 #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Increment13 #29
Conversation
id, err := h.fetchID(c, link) | ||
uniqueErr := false | ||
id, err := h.fetchID(c, user, link) | ||
if strings.Contains(fmt.Sprintf("%v", err), "shortener_short_key") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Сравнение ошибки со строкой лишает нас преимущества проверки типов и чревато ошибками. Правильный путь проверки ошибки описан в учебнике в разделе "Интроспекция и логирование ошибок".
Тут самым простым вариантом будет создание своего типа или своей ошибки в пакете storage, например,
var ErrDupKey = fmt.Errorf("duplicate key")
и проверка на ошибку на уровне хэндлера через errors.Is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кроме того вижу, что код дублируется в функции fetchID
user := getUser(c, h.Key) | ||
body, err := io.ReadAll(c.Request().Body) | ||
if err != nil { | ||
return err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ни в коем случае не стоит возвращать ошибки без дополнительной информации о том, где произошла ошибка. Иначе по ошибке нельзя будет локализовать место, в котором она произошла, либо на локализацию потребуются значительные усилия. Этот момент хорошо описан в учебнике в разделе "Интроспекция и логирование ошибок". Можно воспользоваться стандартным средством fmt.Errorf("read request body: %w", err)
или функцией errors.Wrap
из пакета github.com/pkg/errors
|
||
var id string | ||
id, err = h.fetchID(c, user, link.OriginalURL) | ||
if strings.Contains(fmt.Sprintf("%v", err), "shortener_short_key") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Заменить на кастомную ошибку
|
||
err = h.Storage.SetURL(id, link) | ||
err = h.Storage.SetURL(user, id, link) | ||
if strings.Contains(fmt.Sprintf("%v", err), "shortener_short_key") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Заменить на кастомную ошибку
} | ||
|
||
func (s *Storage) Ping() bool { | ||
db, err := sql.Open("postgres", s.DBCredentials) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Сам ping отсутствует. Нужен вызов метода db.PingContext
No description provided.