Skip to content
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

HW 4 is completed #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

HW 4 is completed #4

wants to merge 2 commits into from

Conversation

timersha
Copy link
Owner

@timersha timersha commented Jan 5, 2025

Домашнее задание №4 «LRU-кэш»

Чек-лист студента (Что это?)

Критерии оценки

  • Пайплайн зелёный - 4 балла
  • Добавлены новые юнит-тесты для списка - 1 балл
  • Добавлены новые юнит-тесты для кэша (включая тест на логику
    выталкивания из кэша редко используемых элементов) - до 3 баллов
  • Понятность и чистота кода - до 2 баллов

Зачёт от 7 баллов

@timersha timersha changed the title hw04_lru_cache HW 4 Jan 9, 2025
@timersha timersha changed the title HW 4 HW 4 is completed Jan 9, 2025
Copy link
Collaborator

@agneum agneum left a comment

Choose a reason for hiding this comment

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

Спасибо за выполненное задание. Хорошая работа
Итого: 8 баллов из 10. Принято

Copy link
Collaborator

Choose a reason for hiding this comment

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

Прошу, чтобы PR содержал только файлы, относящиеся к текущему ДЗ

Comment on lines +51 to +54
v = lruC.queue.PushFront(value)
v.Value = &CacheItem{key, value}
lruC.items[key] = v
lruC.cleanOutdatedElements()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Формально корректнее сначала проверить размер списка, а затем добавлять элемент, чтобы не превысить лимит

}

func (lruC *lruCache) Clear() {
l := lruC.queue.Len()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Не хватает лока на время очистки

Comment on lines +73 to +76
for range l {
delete(lruC.items, lruC.queue.Front().Value.(*CacheItem).k)
lruC.queue.Remove(lruC.queue.Front())
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Выполняется полный перебор списка.
Рекомендовал бы вместо этого переинициализировать поля списка как при создании

"strconv"
"sync"
"testing"

"github.com/stretchr/testify/require"
"golang.org/x/exp/rand"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Почему заменён пакет rand?

newFront := &ListItem{
Value: v,
Next: l.front,
Prev: nil,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Поля с zero-value можно не указывать явно

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