1. Си / Говнокод #17096

    +135

    1. 1
    2. 2
    3. 3
    4. 4
    5. 5
    6. 6
    7. 7
    8. 8
    9. 9
    static enum rc (*request_functions[])(void) = {
    ko,
    koko,
    kokoko,
    illegal_request
};
static inline enum rc illegal_request(void) { return ILLEGAL_REQUEST; }

reply.rc = request_functions[cmd.opcode < NKEYS(request_functions) ? cmd.opcode : ILLEGAL_REQUEST]();

    Вызываем функцию по опкоду с абортом в случае index_out_of_bounds.

    Запостил: codemonkey, 12 Ноября 2014

    Комментарии (38) RSS

    • ava Анонимус 12.11.2014 13:54 # 0

      Poorman's method dispatching.

      Но вообще молодцы что возвращают ILLEGAL_REQUEST а не выполняют какой-то случайный код на который там дальше в памяти указатель случайно оказался
      Ответить

      • ava Dummy00001 12.11.2014 14:09 # 0

        > Poorman's method dispatching.

        лэйбл "poor man's" неприменим. потому что в С нет другуго способа. только свитч/кейс и таблица функций. на практике, таблица функций 20-30% быстрее.
        Ответить

        • ava Анонимус 12.11.2014 14:17 # 0

          Я имел ввиду что с точки зрения современных языков это poorman's.

          Для сишников наверное годно собрать в памяти указатели на функции и по ним скакать
          Ответить

          • ava bormand 12.11.2014 14:37 # 0

            > Для сишников наверное годно собрать в памяти указатели на функции и по ним скакать
            Ты так говоришь, как-будто питонисты не собирают функции в мап, чтобы по ним скакать.
            Ответить

            • ava Анонимус 12.11.2014 14:39 # 0

              разве это не считается у питонистов говнокодом?
              Ответить

              • ava bormand 12.11.2014 14:40 # +1

                Да хрен их знает. Мутить ради этого иерархию классов может оказаться еще большим ГК.
                Ответить

                • ava Анонимус 12.11.2014 14:46 # +1

                  а зачем мапа-то?
                  В конце концов можно сделать так
                  class MessageServer:
    def hide(self):
        print("hide")

    def seek(self):
        print("seek")

    def accept(self, message):
        if message in dir(self):
            getattr(self, message)()
        else:
            print("Unknown command")


message_server = MessageServer()
message_server.accept("seek")
message_server.accept("hide")
message_server.accept("fuck")
                  Ответить

                  • ava bormand 12.11.2014 14:49 # 0

                    > зачем мапа-то
                    Чтобы из разных мест понарегать туда 100500 функций-обработчиков. Может быть даже в рантайме их добавлять при подгрузке каких-нибудь плагинов или конфигов...

                    А ваш вариант, простите, вообще какое-то говно. Ибо позволяет дергать любые методы MessageServer'а - и "приватные" и унаследованные...
                    Ответить

                    • ava Анонимус 12.11.2014 14:51 # 0

                      манкипатчить же можно
                      для кода сверху

                      # ya pluginko
def oppa():
    print("oppa")

setattr(message_server, "oppa", oppa)
                      Ответить

                      • ava bormand 12.11.2014 14:59 # 0

                        Ну не знаю. Жаваскрипт вэй какой-то... Использование класса исключительно в качестве мапа. Имхо, говно. Хотя для каких-то применений может и прокатит.
                        Ответить

                        • ava Анонимус 12.11.2014 15:03 # 0

                          ))а лямбда в мапе лучше?

                          ну ок, я напишу так:
                          message_server.install_new_handler(oppa)

                          всё еще джсвей?
                          Ответить

                          • ava bormand 12.11.2014 15:07 # 0

                            Ну вот так уже лучше. Т.к.
                            1) лишние функции/свойства недоступны (что спасёт от уязвимостей, если в accept прилетают внешние данные)
                            2) класс используется по назначению и содержит мап, а не используетя в качестве оного
                            3) можно прикрутить декоратор для удобной регистрации (см. ниже)
                            4) можно получить список доступных обработчиков (что при использовании класса как мапа превращается в говнище)
                            5) у этого объекта могут быть нормальные методы, которые не смешиваются в кучу с обработчиками

                            P.S. Я надеюсь,что install_new_handler не написан через setattr? :)
                            Ответить

                            • ava Анонимус 13.11.2014 09:25 # 0

                              Я тоже надеюсь что он хранит обработчики внутри себя инкапсулируя знание об их расположении)

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

                      • ava bormand 12.11.2014 15:03 # 0

                        P.S. Можно же сделать декоратор, который внесет в функцию в реестр обработчиков:
                        @command("oppa")
def oppa():
    print("oppa")
                        Ответить

                • ava Dummy00001 12.11.2014 15:05 # +2

                  > Мутить ради этого иерархию классов [...]

                  ... называется энтерпрайз!!!
                  Ответить

              • ava bormand 12.11.2014 15:13 # 0

                P.S. Ну вот у меня была на работе задачка - надо написать демона с JSON RPC, который принимает несложные команды (их штук 30, и они разбиты на группы), исполняет их и отправляет ответы.

                При старте демон сканил папку, искал в ней модули и загружал их. Каждый модуль регал один или несколько хендлеров в глобальную мапу.

                При запросе демон искал хендлер в мапе и исполнял его.

                Говно, как считаете?
                Ответить

                • ava Xom94ok 12.11.2014 23:54 # +2

                  Говно считает, что норм, раз описание решения уложилось в три строки. Инженеры и программисты всегда найдут к чему прикопаться :)
                  Ответить

                  • ava bormand 13.11.2014 00:01 # +1

                    > уложилось в три строки
                    Да там и кода то ненамного больше. Строчек 50-100 от силы в main'е. Остальное в тех самых модулях.

                    P.S. Сейчас бы я поюзал flask и не ебал бы мозг с самодельным RPC. А тогда бакой был ;(
                    Ответить

                  • ava bormand 13.11.2014 00:03 # +1

                    > Говно, как считаете?
                    Блин, а ведь выглядит как обращение. Походу, я лох и обосрался с пунктуацией. Сорри :)
                    Ответить

          • ava Dummy00001 12.11.2014 15:13 # 0

            > Я имел ввиду что с точки зрения современных языков это poorman's.

            Оно и на современных языках часто оказывается нужно. Например, С++ vs Object Pascal, диспатч через С++ виртуальный метод был на порядок быстрее чем в Дельфе, почему приходилось в узких местах в ручную таблицы указателей на методы делать, с диспатчем не сильно отличающимся от того как в ГК.
            Ответить

    • ava Dummy00001 12.11.2014 14:05 # 0

      `enum rc` это само по себе достойно отдельного ГК. видел несколько раз попытки rc в энум засунуть - все заканчивались весьма спектакулярным провалом. но в начале выглядит всегда отлично.

      Ответить

      • ava bormand 12.11.2014 14:42 # 0

        > static enum rc
        Ну хотя бы тайпдеф то можно было сделать... Чтобы enum не писать каждый раз, и была возможность перепилить на uint32_t...
        Ответить

        • ava kipar 12.11.2014 15:15 # 0

          Кстати часто вижу что не любят сишники тайпдефов.
          Например у атмела:
          static inline void adc_set_conversion_trigger(struct adc_config *conf,
		enum adc_trigger trig, uint8_t nr_of_ch, uint8_t base_ev_ch)

          видимо считают что так удобнее, сразу видно где у нас запись, а где енум. Других объяснений не вижу.
          Ответить

      • ava codemonkey 12.11.2014 16:11 # 0

        Предлагаете с сраными #define-ами их расписывать?
        Ответить

        • ava Dummy00001 12.11.2014 16:27 # 0

          определять константы для возврата через enum это одно.

          но делать сам тип кода возврата enum'ом это совсем другое.

          с другой стороны, у "сраных дефайнов" есть Н-ое количество преимуществ, в оссобенности для больших проектов. например можно проверить определен ли уже код ошибки или нет. в добавок, синтакс не требует этих грёбаных запятых, которые при переливании кодов из одного языка в другой надо выкидывать. распилить кучу дефайнов я могу и на шелле - но что бы распилить кучу enum'ов как минимум перл желателен.
          Ответить

          • ava codemonkey 12.11.2014 16:35 # 0

            >но делать сам тип кода возврата enum'ом это совсем другое.
            Обоснуйте.
            Ответить

            • ava Dummy00001 12.11.2014 16:46 # +1

              это подразумевает центализованое определение данного энума и его значений.

              это работает только на простых проектах, на которых внутренний rc это просто overdesign.

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

              • ava codemonkey 12.11.2014 17:05 # 0

                Проект маленький и больше 20 rc там не будет в принципе.
                Ответить

    • ava TarasB 12.11.2014 14:07 # 0

      что означает конструкция enum smth(sometype) {...} ?
      Ответить

      • ava TarasB 12.11.2014 14:14 # +1

        я понял
        ёбаная ритчи уебанское это опять объявление сука
        Ответить

        • ava TarasB 12.11.2014 14:15 # +1

          request_functions : array [] of rc() = {...}
          Ответить

          • ava Dummy00001 12.11.2014 15:09 # 0

            да.

            сишные enum/struct/union (захардкоженые в язык) это прародители современных нэймспэйсов (юзер дефайнабл).

            на практике весьмя удобно (хотя и очень ограничено), потому что нет конфликта между именами переменных и именами типов.
            Ответить

      • ava Анонимус 12.11.2014 14:16 # 0

        Если я помню сишечку то
        enum[ТИП]{}
        [ТИП] в данном случае есть указатель на функцию которая ничего не принимает (void)
        Ответить

    • ava gumbert 13.11.2014 13:52 # +1

      Печально, что ILLEGAL_REQUEST используется как opcode. Я бы выпилил illegal_request и заменил последнюю строку на

      reply.rc = cmd.opcode < NKEYS(request_functions) ? request_functions[cmd.opcode]() : ILLEGAL_REQUEST;
      Ответить

      • ava codemonkey 13.11.2014 18:46 # 0

        >ILLEGAL_REQUEST используется как opcode
        Не совсем. В энуме опкодов его нет.

        Кстати, ваш код идентичен ГК если раз-inline-ить функцию illegal_request.
        Ответить

        • ava bormand 13.11.2014 18:48 # 0

          > В энуме опкодов его нет
          Но, тем не менее, он равен числу опкодов + 1. И вот это уже пиздец...

          В енуме rc лежит такое значение, которое равно количеству элементов +1 в енуме опкодов.

          Вариант gumbert'а лучше, т.к. не вносит неявной связи между двумя енумами.
          Ответить

    Добавить комментарий