Пре-релиз aiomongowire, комментарии/замечания/предложения

Предложение: запихать информацию из поста в README.

Так и сделаю, было лень

По файлу _base_op.py

Вместо

    @classmethod
    @abc.abstractmethod
    def op_code(cls) -> OpCode:
        pass

    @classmethod
    @abc.abstractmethod
    def has_reply(cls) -> bool:
        """
        True if operation is expected to return something
        """
        pass

Достаточно

    op_code: OpCode
    has_reply: bool

Далее

    @classmethod
    def from_data(cls, op_code: OpCode, data: io.BytesIO) -> 'BaseOp':
        """
        Deserialize operation from bytes
        :raises KeyError: If operation has unknown OpCode
        """

KeyError очень не рекомендую использовать, особенно для таких случаев. Так как он часто используемый, то может быть отловлен в самых неожиданных местах, если забудут обернуть вызов from_data в try-except. Стоит объявить и использовать отдельный класс исключения.

    @classmethod
    def from_data(cls, op_code: OpCode, data: io.BytesIO) -> 'BaseOp':
        """
        Deserialize operation from bytes
        :raises KeyError: If operation has unknown OpCode
        """
        return _OP_CLASSES_BY_CODE[op_code]._from_data(data)

    @classmethod
    @abc.abstractmethod
    def _from_data(cls, data: io.BytesIO):
        """
        Deserialize operation from bytes. Internal implementation.
        """
        pass

from_data - стоило бы или переименовать, или вынесте вообще за пределы класса. Он явно тут лишний.

В свою очередь _from_data надо сделать публичным. Тогда выглядит адекватнее.

Общие замечания.

• Использование классметодов, за исключением инстанцирования (from_data), обычно плохой признак.

• Можно много где использовать dataclasses.dataclass, вместо использование обычного класса и __slots__ = [...]

Спасибо за комменты, поправлю. Не согласен только с двумя: from_data и датаклассами.

Для from_data имхо можно просто переопределять её внутри класса вместо создания _from_data ( оставить один метод для базы и сабклассов). Та, что в базе - обычный фасад, и ставить её куда-то ещё кмк странно

Про датаклассы - не могу сказать что я гонюсь за скоростью (на питоне лол), но вот датаклассы реально медленные, а профит от них весьма сомнителен в данном случае. Они хороши когда в классе нет кастомной логики сериализации и надо просто жсон/dict переложить, но в данном случае кажется они максимум уберут init, и то не факт.

Для from_data имхо можно просто переопределять её внутри класса вместо создания _from_data ( оставить один метод для базы и сабклассов). Та, что в базе - обычный фасад, и ставить её куда-то ещё кмк странно

Не распарсил. Лучше кодом.

датаклассы реально медленные

Возможно. Но питон вообще не быстрый.

Возможно. Но питон вообще не быстрый

есть такое дело. Но делать намеренно медленнее без какого-либо профита странно. Основной профит датаклассов это когда у тебя нет никакой сериализации сложнее жсона, нет правил, бинарщины, есть тупо объект с полями. В данном случае скажем bytes оно не снимет, и from_data тоже. Максимум снимет init, но это минус одна строка по факту (собственно сигнатура). Если использовать хотя бы frozen, дефолтные фабрики, или тулы типа pydantic/dacine - тогда понятно. А так - не стоит оно того.

Не распарсил. Лучше кодом

можно сделать так

class Base:
    @classmethod
    def from_data(data, opcode = None):
        return SUBCLASSES[opcode].from_data(data)

class Child
    @class
    def from_data(data, opcode = None):
        return ....

Но это некрасиво ввиду наличия параметра, на который все остальные методы дружно забьют болт. Да еще и контр-интуитивно, дали один опкод, а вышел другой. В Яве такая конструкция работает легко и бодро ввиду нормального overload, исключающего второй параметр для сабклассов. В питоне - либо разные имена методов, либо **kwargs, за который кмк надо расстреливать.

По поводу убрать его из класса - как ты сам сказал, «за исключением инстанцирования», на вид это как раз его место, хз. Но если есть идеи где оно будет лежать лучше - покажи, будет интересно

Ну и про _from_data - по идее без опкода ты не можешь (не должен) знать что там тебе пришло, даже в ответе базы может быть или op_reply, или op_compressed если сжатие включено. Так что наличие единственной публичной точки входа в BaseOp это как раз намеренно (в т.ч. чтоб не провоцировать попытки гадания по байтам).

**kwargs, за который кмк надо расстреливать

аргументы?

можно сделать так

А зачем? Это странно.

Я имел в виду что метод

    @classmethod
    def from_data(cls, op_code: OpCode, data: io.BytesIO) -> 'BaseOp':
        return _OP_CLASSES_BY_CODE[op_code]._from_data(data)

должен быть или методом класса-фабрики, или просто функцией внешней.

class OpFactory:
    def from_data(self, op_code: OpCode, data: io.BytesIO) -> 'BaseOp':
        return _OP_CLASSES_BY_CODE[op_code].from_data(data)

Ну и про _from_data - по идее без опкода ты не можешь (не должен) знать что там тебе пришло

Какая разница, что там с опкодом? Опкод тебе дает какие-то гарантии особые? :) Считай, что пользователь класса всегда знает какой там опкод. Или какой-тип сообщения.

аргументы?

Ты серьёзно? Это наверное топ самого неподдерживаемого шлака в питоне, после разве что getattr

Ну, в целом да, наверно ты прав. Ок, попробую сделать посмотреть как выйдет

методом класса-фабрики

не надо тащить джаву в питон и городить классы ради классов. в этом нет смысла

просто функцией внешней

это нормальный подход

В чём твоя проблема с **kwargs и getattr? почему неподдерживаемый? что ты предлагаешь вместо этого?

Ты либо троллишь, либо не работал с проектами крупнее сабжа и с количеством разрабов больше одного. На 1к строк да, можно и kwargs и getattr. На 100к строк ты офигеешь искать что куда идёт и почему в метод передали всякий хлам вместо аргументов. Про 1кк строк вообще молчу. Так писали в самом начале второй ветки, которая уже, к счастью, сдохла

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

Единственный случай когда использование kwargs можно аргументировать - прокся между двумя вызовами сторонней либы, когда тебе скажем нужно тупо вставить переменную в контекст и тебе в целом побоку что там передали в вызов т.к. ты просто его проксируешь дальше. Пример - middleware в aiohttp.

Все остальное это просто кривая архитектура, а не необходимость

А как насчёт декораторов?

Декоратор - такая же прокся. Ключевая часть:

когда тебе в целом побоку что там передали в вызов т.к. ты просто его проксируешь дальше

Если тебе не побоку и ты используешь что-то что передали, и тем более если это ещё и твой собственный код - kwargs это самое дно куда только можно опуститься, и даже наличие доки что там за keywords этого не исправит

Ну так оно и не нужно в 99.9% случаев. Но иногда его не избежать. И тогда я не вижу ничего плохого в применении этих фич.

немного не по теме, просто интересно стало. а для чего все файлы с подчеркивания начинаются? это какая-то конвенция? я просто на питонах уже почти 15 лет не пишу (было лет 5 назад немного кодинга на нем, но это не считается), не знаю в какую он сторону развился

Подчеркивание - типа private модули. На самом деле конечно нет, и их все равно можно импортировать, но штатный code completion их не покажет по умолчанию, и ide будет ругаться что нефиг их юзать. Всё что публичное - вынесено в init в корне

В дополнение - грубо говоря это чтоб импорт делали aiomongowire.SomeClass, а не aiomongowire.package.another.SomeClass. Ну и чтоб выставить наружу определённый интерфейс и скрыть остальное