Няшная / Говнокод #2968 Ссылка на оригинал

0

  1. 1
  2. 2
  3. 3
  4. 4
  5. 5
  6. 6
  7. 7
  8. 8
  9. 9
new = ngx_pnalloc(cf->pool, sizeof("; domain=") - 1 + domain->len);
    if (new == NULL) {
        return NGX_CONF_ERROR;
    }

    p = ngx_cpymem(new, "; domain=", sizeof("; domain=") - 1);
    ngx_memcpy(p, domain->data, domain->len);

    domain->len += sizeof("; domain=") - 1;

Это, между прочим, файл nginx-0.7.62\src\http\modules\ngx_http_userid_ filter_module.c.
Такие вот разочарования случаются...

Запостил: dmitry_113 dmitry_113, (Updated )

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

  • ...имелось в виду захардкоженное 4-кратное повторение "; domain=", если кто не понял.
    Ответить
    • все равно значение sizeof("; domain=") будет вычислено на этапе компиляции и вставлено как константа.
      Ответить
    • с другой стороны, какая разница: напишет он четыре раза "; domain=" или напишет он четыре раза какое-нибудь "DOMAIN_PATTERN"? и там и там много буков. разницы особой чисто с технической стороны большой нет )
      Ответить
      • разница в том, что менять придётся в 4 местах... хотя, конечно, если он пользуется емаксом или вимом это займёт всего на пару движений побольше.

        Но вообще всё это придирки, не говнокод.
        Ответить
  • А разве new - не ключевое слово?
    З.Ы.Можно минусовать.
    к-своему-стыду-забывающий-Си-и-С++-кун
    Ответить
    • В Си - нет, не ключевое.
      Грабли возникнут, только если попытаться скомпилировать это компилятором С++.
      Ответить
  • повторяться не очень хорошо конечно, я бы сам вывел в константу отдельную, но в данном случае совершенно некритично...
    Ответить
  • Да вот нихрена имхо не нужно тут константы -- по ее названию все равно не поймешь, что там такое, т.е. она только если усложнит код, т.к. нужно будет вспоминать где там ; стоит, если когда-нить придется код править + как уже говорили компилятор все равно один раз память под строку выделит, так что выигрыша вообще ни в чем не получится.
    Ответить
    • > если когда-нить придется код править + как уже говорили компилятор все равно один раз память под строку выделит, так что выигрыша вообще ни в чем не получится.

      тут дело не в памяти вообще-то. если чуваку придётся изменить это значение, он может забыть в одном месте что-нибудь изменить. т.е. в одном месте будет "; newdomain=", а в другом может остаться "; domain=".

      в понятиях маинтабилити, это тоже самое, что писать some_code(0, SOME_CONSTANT); some_code(1, SOME_CONSTANT); some_code(2, SOME_CONSTANT); вместо for(i = 0; i < 3; i++) { some_code(i, SOME_CONSTANT); }

      это неявный копипаст, что чревато чисто человеческими ошибками (компилер тут ни при чём)

      поэтому выигрыш для вынесения в константу есть!

      а минусуют в основном потому что "nginx - это круто и выше моего понимания, потому что я быдлокодер на жава, nginx-то уж писали тру-хоцкеры, которые не могут ошибиться"
      Ответить
      • Ненене, я это все понимаю =), и это оправдано, когда константа используется более, чем в одной функции, но когда это именно локальное константное значение -- то при встречаемости < 5 на функцию, я не завожу локальной static const переменной, ну это кагбэ сугубо имхо =)
        Ответить
      • Подозреваю, что это стандартная строка и она прям так вдруг не изменится.
        Изменить на одной строке и не заметить на другой нереально, тем более что делается это не в блокноте
        Ответить
        • Проходили уже - вполне может измениться.
          Кому-нибудь "лишний" пробел не понравится, или еще что.
          Опять же, люди разные - один заменит везде, другой скажет "я типа не заметил, что их там много".
          Ответить

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

Где здесь C++, guest?!

    А не использовать ли нам bbcode?


    8