1. C++ / Говнокод #2419

    +66.6

    1. 1
    2. 2
    3. 3
    4. 4
    5. 5
    6. 6
    7. 7
    //Дефайн в некотором хэдере
    #define _TAPI(b) do {BOOL _b = (BOOL)(b); if (!b) throw(system_exception(GetLastError())); } while (false);
    //...
    // И далее такое:
    //...
    _TAPI (::CreateProcess(0, (LPWSTR)m_process.c_str(), 0, 
    				0, FALSE, CREATE_SUSPENDED, 0, 0, &si, &pi));

    Мой говнокод, хотя скорее опечатка =), я с такими явлениями уже не раз сталкивался, но все равно в течение получаса не мог понять, почему у меня по два процесса запускается =)))

    Запостил: ISith, 15 Января 2010

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

    • Использование препроцессора с нехилыми конструкциями и корявыми названиями - истинный говнокод.
      Ответить
      • Это проблема лишь эстетическая и субъективная..

        В первую очередь ругаться, IMHO, надо лишь на сам алгоритм... А эстетические проблемы существуют в любом коде.
        Ответить
    • TAPI - Microsoft Telephone API? Ну и название...
      Ответить
      • Зачем такое странное название? Хоть можешь сказать как оно в твоей проге расшифровывается?
        Такое название - уже говнокод.
        Ответить
        • throw api =) также есть _THANDLE, _TTRUE и т.д. да я и сам думаю, что названия не ахти.. скорее всего поменяю, в частности уберу T т.к. все такие макросы будут бросать эксепшн, а изначально планировалось, что будут и просто возвраты =)
          Ответить
    • >я с такими явлениями уже не раз сталкивался
      Тоесть такой код повсеместно?
      Ответить
    • >while (false)
      А зачем цикл, но выполняющийся один раз?
      Ответить
      • чтобы имена переменных не конфликтовали, можно было бы просто скобки -- не принципиально, если break не юзается
        Ответить
        • Не нужно это.
          #define _TAPI(b) if (!((BOOL)(b))) throw(system_exception(GetLastError()));
          Вполне себе справится.
          Ответить
          • "do { .... } while(0)" имеет туже семантику что и вызов void функции. т.е. твой висячий if будет работать с граблями в когтексте типа:

            if (condition)
            _TAPI( Call1(...) );
            else
            _TAPI( Call2(...) );

            do{}while(0) это старый трюк...
            Ответить
            • Значит просто напиши:
              #define _TAPI(b) {if (!((BOOL)(b))) throw(system_exception(GetLastError())); }
              И нет не какого висячего ифа.
              Ответить
    • харэ уже постить все подряд. Может еще coldfusion откроем и будем охуевать - что за херня, я нихуя не понимаю, запощу ка на гк!
      Ответить
      • Не интересно, не читай. Некоторые люди на говнокоде учаться находить говнокод и не писать такой же.
        Ответить
    • Где опечатался то?
      Странно. Такой говнокод и минусуют...
      Ответить
      • >Странно. Такой говнокод и минусуют...
        А тут так часто. Не поняв код записывают его в "неговно", минусуя.
        А нужно тот код, что непонимаешь - сразу в говнокод запихивать, ибо код для людей и должен быть понятен в команде разаработчиков. А машина, если дивайс не встроенный, то все схавает. Да и оптимизаторы сейчас хорошие.
        Ответить
    • m_process что сдесь значит?
      Ответить
    • >BOOL _b = (BOOL)(b);
      ХаХаХа. )))

      Чего не так сделал?
      #define _TAPI(b) if (!((bool)(b))) throw(system_exception(GetLastError()));
      Хотя тоже говно...
      Ответить
      • кстати, (bool)(b) всяко хуже, чем (BOOL) т.к. нужно конвертировать потенциально 4-байтное b в однобайтный 0 или строго 1 =), обратная же конвертация менее болезненна =)
        Ответить
        • Откуда знаешь, что обратная конвертация менее болезненна? Может быть в обоих случаях не больно?
          Не нужно такого утверждать, пока непрошёлся профлаером. Только узкие места требуют оптимизации. Сомневаюсь, что создание процесса - узкое место. Создание процесса в сотни раз дольше, чем конвертация из байта в 4 байта и обратно.
          Улучшай алгоритм, а не делай бесполезную работу, котрую сделает оптимизатор. (с)
          Ответить
          • >>Откуда знаешь, что обратная конвертация менее болезненна? Может быть в обоих случаях не больно?
            Я реверсер по совместительству =) поэтому отлично знаю что во что компилируется =). А про не оптимально это я просто к слову сказал. Кстати по поводу каста BOOL->bool даже mvs компилятор варнинг дает =)
            >>Улучшай алгоритм, а не делай бесполезную работу, котрую сделает оптимизатор. (с)
            полностью согласен, от себя добавлю: "..может ничего оптимизировать вообще нахуй не стоит и никто этого не заметит"
            Ответить
          • >>Откуда знаешь, что обратная конвертация менее болезненна? Может быть в обоих случаях не больно?
            Откуда знаешь, что не болезненна? Ты на то и программист, чтобы твой софт работал нормально, то есть нужно проверить и говорить точно, а не "сомневаюсь". Сколько уже таких на оптимизатор надеялись, ипаааать...
            Ответить
            • >Откуда знаешь, что не болезненна?
              Знаю, потому что не больно.
              >Ты на то и программист, чтобы твой софт работал нормально
              Он будет работать гораздо хуже, если я оптимизирую в ущерб читаемости кода(качества кода), а это значит, что исправлю меньше ошибок и удленню разработку.
              >Сколько уже таких на оптимизатор надеялись, ипаааать...
              И сколько же? Разве для каких-то мат вычислений критично, а для всего остального - не нужно. Например, какая может быть оптимизация в данном случае при создании процесса? Идиотизм. Не нужна она там. Лучше меньше строчек кода написать.
              Ответить
    • >> Где опечатался то?
      #define _TAPI(b) do {BOOL _b = (BOOL)(b); if (!_b) throw(system_exception(GetLastError())); } while (false);

      >> m_process что сдесь значит?
      да это совершенно не важно в контексте говнокода =)

      >>Чего не так сделал?
      >>#define _TAPI(b) if (!((bool)(b))) throw(system_exception(GetLastError()));
      >>Хотя тоже говно...

      да просто копипастил из
      #define _TAPI_E(e) do {apierr _e = (e); if (_e) throw(system_exception(_e));} while(false);
      да тож еще фича =)

      а вообще использование таких макросов делает код более читаемым и понятным, чем постоянные ифы.
      Ответить
      • >а вообще использование таких макросов делает код более читаемым и понятным, чем постоянные ифы.
        Неверю.
        Ответить
        • Особенно, когда таких макросов с кривыми названиями тысячи и каждый нужно запомнить, что делает.
          Ответить
        • такой код используется только на самом низком уровне, в обертках над апи функциями и делает его похожим на весь остальной, где кагбэ только эксепшны и никаких проверок возвращаемых значений + этих макросов не тысячи а несколько на самые распространенные проверки, так что я все еще за =)
          Ответить
        • код был бы более читаемым если название макроса не из 5 букаф...
          вот если бы его назвать как нить типа _THROW_IF_FALSE_API (ну неточно ибо я ж весь код не знаю, но смысл в том чтобы название не давало само по себе повода для размышлений человеку который код разбирает, например новому сотруднику и т.п.)

          короче как и большинство вещей все есть палка о двух концах, макросы в том числе...
          Ответить
      • Копипастить вредно, если не умеешь пользоваться. Скопипастеный код нужно исправлять или разбивать на функции, что-бы не было копипасты.
        Ответить
    • >> m_process что сдесь значит?
      >да это совершенно не важно в контексте говнокода =)
      Для меня важно. И да. Такое неговорящее название - тоже говнокод.
      Ответить
      • путь к файлу, который нужно запускать =) Это один из двух членов класса, котрый запускает процесс с инжектом длл, либо инжектит длл в запущенный процесс, куда уж говорящей =)
        Ответить
    • Ой говнокод...
      Ответить
    • Неожиданно, такой говнокод и минусуют?
      Ответить
    • Тут от С++ конечно только system_exception и две точечки
      Ответить

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