Pull to refresh

Comments 6

Хотел бы ознакомиться с исходным кодом, но ссылку на репозитарий библиотеки в статье не нашел. Не могли бы дать?
Спасибо, посмотрел. Маленький ревью кода:

  1. Не ловите все исключения, а только конкретные, те которые нужно отработать;
  2. Не оборачивайте logger во вспомогательные методы или функции, используйте его напрямую;
  3. Лучше сразу упасть при проблеме с импортом, чем потом когда приложение инициализируется;
  4. Надо следить за строгостью оформления кода по PEP8;
  5. Табы в коде Python — это зло;
  6. Если нацелены на большой охват аудитории, то не пишите докстринги на русском;

Если не понятен какой-либо пункт, я постараюсь разъяснить отдельно.

На мой взгляд, код пакета похож на обертку для кода, чем на удобный в использовании инструмент. Если делать почти одинаковые сервисы с использованием этого пакета, то с ростом количества сервисов сложнее будет поддерживать совместимость с этим пакетом. Проще перенести весь этот код в код сервиса и использовать там, благо здесь не много кода.

Если есть вопросы, я с удовольствием отвечу.
Спасибо, за отзыв. Есть вопросы.
2 — почему оборачивать логгер плохо?
3 — тут расчёт на то, что может понадобиться просто приложение, без подключенного функционала редиса/реакта, то есть получается что импортировать тот же aioredis необязательно
по 4,5,6 — вопросов нет, так вышло, тоже буду исправлять в будущем.

3 — следствие того, что Вы в одной библиотеке пытаетесь собрать разный, несвязанный функционал. Слышали такое понятие Unix-way?


Пишите программы, которые делают что-то одно и делают это хорошо

То же применимо к библиотеке. Обертка для редиса — одно, для реакта — другое, логирование — третье. Тогда, если мне нужен будет какой-то функционал, я сделаю pip install torskel-redis,
и не нужны будут костыли вроде use_redis=False. Соответственно, у разных библиотек должны быть разные setup файлы со своими зависимостями


1 — добавлю про исключения. Не делайте try .. except всех исключений. Почему — см. на sof

2 — почему оборачивать логгер плохо?

Он уже готов к использованию, не нужно оберток, он и так простой. Где-то в начале файла мы его получаем и используем в любом месте.
3 — тут расчёт на то, что может понадобиться просто приложение, без подключенного функционала редиса/реакта, то есть получается что импортировать тот же aioredis необязательно

Вам редко надо будет включать или отключать Redis в приложении. Проще aioredis сразу установить и импортировать. Иначе вам придется постоянно проверять переменную на None, если она не пустая только тогда вызывать функцию из пакета.

Вот этим куском вы как бы эмулируете стандартное поведение Python при импорте без обработки исключения ImportError:
try:
    import aioredis
except ImportError:
    aioredis = None
...
           if aioredis is None:
                raise ImportError('Required package aioredis is missing')
            else:
                self.init_redis_pool(loop)
...


Sign up to leave a comment.

Articles