- DecoratorManager название не раскрывает, что делает класс
- $this->logger->critical('Error'); не информативное сообщение в логе, хотя есть $exception, и можно нормально сообщение сделать
- Размытые обязанности у декоратора, он и кеширует (+обязанность уметь делать ключ кеша) и логирует, и при этом т.к. наследник DataProvider, весь функционал и обязанности дата провайдера тоже переходят ему. Нарушение SOLID.
- Логер устанавливается отдельным методом, по дефолту никакого нет, т.е. если я вызову getResponse могу словить вызов $this->logger->critical('Error') на null
- У декоратора должна храниться ссылка на объект, который он декоррирует и индентичный интерфейс, в этом классе DecoratorManager сделано не так, и еще ловится исключение
- Чтобы сохранить кеш надо вызвать $this->cache->save($cacheItem);
- json_encode($input) как ключ, не самый лучший вариант, например в мемкеше некотоые символы не могут использоваться в ключе, это следует учесть.
- Нет возвращаемых типов у функций и тайп хинтинга у параметов функций.
- isHit, я бы заменил на проверку результата get. Возможно ситуация race condition и тогда несколько запросов пойдут обновлять кеш.
-
Notifications
You must be signed in to change notification settings - Fork 0
connorholt/skyeng_review
Folders and files
| Name | Name | Last commit message | Last commit date | |
|---|---|---|---|---|
Repository files navigation
About
No description, website, or topics provided.
Resources
Stars
Watchers
Forks
Releases
No releases published
Packages 0
No packages published