-
Notifications
You must be signed in to change notification settings - Fork 0
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
Dev #1
Dev #1
Conversation
internal/handler/accrual/accrual.go
Outdated
|
||
} | ||
|
||
<-time.After(time.Second * time.Duration(5)) |
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.
[BLOCKER] здесь будет утечка памяти. Лучше использовать time.Ticker и написать как-то так:
t := time.NewTicker(time.Second * time.Duration(5))
defer t.Stop()
for {
select {
case <-t.C:
// logic
}
}
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.
добавил функцию sleep
коммит 60838f1
internal/handler/handler.go
Outdated
func (h *Handler) Balance(w http.ResponseWriter, r *http.Request) { | ||
username := r.Header.Get("username") | ||
|
||
user, err := h.Repository.GetBalance(context.Background(), username) |
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.
[BLOCKER] context нужно брать из Request. Если по какой-то причине клиент отклонит запрос, контекст завершится и запрос в БД выполняться не будет.
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.
fix в коммите 56800de
internal/handler/accrual/accrual.go
Outdated
|
||
func sleep(ctx context.Context, interval time.Duration) error { | ||
timer := time.NewTimer(interval) | ||
select { |
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.
[BLOCKER] Здесь так же стоит добавить defer timer.Stop(). Чтобы после завершения выполнения функции, все ресурсы timer освободились.
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.
удалил функцию
коммит 2f5f087
internal/handler/accrual/accrual.go
Outdated
|
||
} | ||
|
||
if err := sleep(ctx, time.Duration(5)*time.Second); err != nil { |
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.
[LINT] выглядит чуть-чуть не красиво. Можно оставить как есть, но обычно так не пишут. Если не хочешь видеть select
не обязательно делать для этого отдельную функцию. Можно просто написать:
func SendAccrual() {
ticker := time.NewTicker(time.Duration(5)*time.Second)
defer ticker.Stop()
for _ = range ticker.C {
// do something
}
}
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.
переписал цикл с использованием ticker
коммит 2f5f087
internal/handler/handler.go
Outdated
return | ||
} | ||
|
||
tkn, _ := util.CreateToken(user.Login, user.ID) |
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.
[LINT] если CreateToken вернет ошибку, то лучше ее обработать и вернуть пользователю ошибку
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.
обработал ошибку
коммит 2f5f087
No description provided.