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

    +154

    1. 1
    2. 2
    3. 3
    4. 4
    if (request->status().is_success() &&
          (request->GetResponseCode() / 100) == 2) {
          /* блаблабла */
      }

    Было случайно откопано в исходниках хромиума (http://src.chromium.org/svn/trunk/src/webkit/appcache/appcache_update_job.cc). И первый вопрос который возникает это "Ну вот нахера???".

    Запостил: POPSuL, 11 Марта 2011

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

    • request->GetResponseCode() >= 200 && request->GetResponseCode() < 400
      т.к. нужны коды 2хх и 3хх => оптимизация
      Ответить
      • 3хх - откуда ?
        Ответить
        • да, конец рабочей недели , ваш комментарий верен а мой нет (посыпает голову пеплом)
          Ответить
    • ложь
      оу, это же целочисленное, я позабыл..
      Ответить
    • Ну, как сказать : чтобы коды от 200 до 299 включительно удовлетворяли условию.
      Ответить
    • ох уж эти оптимизаторы
      Ответить
    • ИМХО, такая оптимизация даст выигрыш только в случаях, когда код используется многократно (например, в циклах) или если метод является "узким местом" и каждая миллисекунда на вес золота. Во всех остальных случаях такой код только раздражает, если нужно быстро в нем разобраться и поставить диагноз
      Ответить
      • Это не оптимизация, это сделано для читаемости.
        Ответить
        • По-моему, читабельнее было бы (code >= 200 && code <300), но тут либо два вызова надо делать, либо дополнительную переменную заводить.
          Ответить
          • Вот-вот, дополнительную переменную, лишнюю строчку, кучу скобок для группировки.

            Между тем, проблема как стоит? Обработать коды вида 2xx -- т.е. трёхцифровые, первая цифра (сотни) равна 2. Эта запись -- и есть буквальная (и наиболее естественная, с моей точки зрения) запись решения.

            Может быть на другом языке можно было бы записать проще: request->GetResponseCode() ~ /^2..$/, но в C++ это было бы громоздко и глупо.
            Ответить
          • вообще надо бы прояснить что делает is_success()
            потому как наблюдается тавтология
            ибо когда статус-код равен 2xx - запрос был successful
            Ответить
            • Может быть это означает, что какой-то ответ от сервера вообще пришёл и он соответствует http?
              Ответить
            • Глядя на исходник по ссылке, похоже, что is_success() это более общее, а код - более частное.
              Например встречается такое:
              if (request->status().is_success()) {
                  response_code = request->GetResponseCode();
                  is_valid_response_code = (response_code / 100 == 2);
                  request->GetMimeType(&mime_type);
                  is_valid_mime_type = (mime_type == kManifestMimeType);
                }
              Ответить
      • в этом случае надо делать тогда так:
        code = request->GetResponseCode();
        if ((code >= 200) && (code < 300))
        но уж точно не делить на 100 и сравнивать с 2.
        Ответить
        • одно деление и одно сравнение против двух сравнений, коньюкции и еще одного неявного сравнения
          Ответить
          • > и еще одного неявного сравнения
            Что это? Где оно?
            Ответить
          • в скомпилированном варианте будет 2 последовательных сравнения с условными переходами.
            и это в любом случае лучше,чем деление + сравнение.
            Ответить
        • скобки лишние
          Ответить

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