Skip to content

Conversation

combodga
Copy link
Owner

@combodga combodga commented Oct 7, 2022

No description provided.

id, err := h.fetchID(c, link)
uniqueErr := false
id, err := h.fetchID(c, user, link)
if errors.Is(err, h.Storage.ErrDupKey) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я бы предложил упростить эту логику, оставив

if err != nil && !errors.Is(err, h.Storage.ErrDupKey) {
  return fmt.Errorf("fetch id: %w", err)
}

l := &Link{...}

if errors.Is(err, h.Storage.ErrDupKey) {
  return c.JSON(http.StatusConflict, l)
}

return c.JSON(http.StatusCreated, l)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Потому что обнуление ошибки может привести к неожиданным последствиям, если логика будет меняться/дорабатываться

_, err = db.Exec("INSERT INTO shortener VALUES ($1, $2, $3, FALSE)", user, id, link)
if err != nil {
if err, ok := err.(*pq.Error); ok {
if err.Code == "23505" {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Стоит вынести в константу с говорящим названием

s.Pairs[user][id] = link

if s.DBCredentials != "" {
db, err := sqlx.Connect("postgres", s.DBCredentials)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я думаю, что нет смысла каждый раз подключаться к базе, можно держать объект db в структуре Storage

for _, linkToDelete := range l {
for savedLink := range list {
if savedLink == linkToDelete {
go h.Storage.UpdateURL(user, linkToDelete, true)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Предлагаю воспользоваться советом из учебника и потренироваться в написании FanIn.
Для этого можно запустить отдельную функцию в фоновой горутине, которая принимает канал со ссылками, читает из него пачку ссылок и удаляет их одним запросом.
Здесь же в DeleteURL вместо запуска дополнительной горутины можно просто положить ссылку в этот канал.
Такой подход имеет два достоинства:

  • снизится число отдельных запросов к базе данных
  • снизится число единовременно запущенных горутин (сейчас число горутин = число единовременных запросов на удаление * число ссылок в рамках одного запроса, а будет - число единовременных запросов на удаление + 1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants