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

    • ava theli 11.03.2011 17:39 # −1

      request->GetResponseCode() >= 200 && request->GetResponseCode() < 400
      т.к. нужны коды 2хх и 3хх => оптимизация
      Ответить

      • ava absolut 11.03.2011 17:51 # 0

        3хх - откуда ?
        Ответить

        • ava theli 11.03.2011 18:13 # +2

          да, конец рабочей недели , ваш комментарий верен а мой нет (посыпает голову пеплом)
          Ответить

    • ava mindfreakthemon 11.03.2011 17:40 # −2

      ложь
      оу, это же целочисленное, я позабыл..
      Ответить

    • ava absolut 11.03.2011 17:41 # +2

      Ну, как сказать : чтобы коды от 200 до 299 включительно удовлетворяли условию.
      Ответить

    • ava ddkprog 12.03.2011 02:38 # +1

      ох уж эти оптимизаторы
      Ответить

    • ava g00d 12.03.2011 10:26 # 0

      ИМХО, такая оптимизация даст выигрыш только в случаях, когда код используется многократно (например, в циклах) или если метод является "узким местом" и каждая миллисекунда на вес золота. Во всех остальных случаях такой код только раздражает, если нужно быстро в нем разобраться и поставить диагноз
      Ответить

      • ava gegMOPO4 12.03.2011 12:27 # 0

        Это не оптимизация, это сделано для читаемости.
        Ответить

        • ava absolut 12.03.2011 12:43 # 0

          По-моему, читабельнее было бы (code >= 200 && code <300), но тут либо два вызова надо делать, либо дополнительную переменную заводить.
          Ответить

          • ava gegMOPO4 12.03.2011 13:04 # 0

            Вот-вот, дополнительную переменную, лишнюю строчку, кучу скобок для группировки.

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

            Может быть на другом языке можно было бы записать проще: request->GetResponseCode() ~ /^2..$/, но в C++ это было бы громоздко и глупо.
            Ответить

          • ava bugmenot 12.03.2011 13:06 # 0

            вообще надо бы прояснить что делает is_success()
            потому как наблюдается тавтология
            ибо когда статус-код равен 2xx - запрос был successful
            Ответить

            • ava gegMOPO4 12.03.2011 13:22 # 0

              Может быть это означает, что какой-то ответ от сервера вообще пришёл и он соответствует http?
              Ответить

            • ava absolut 12.03.2011 13:24 # 0

              Глядя на исходник по ссылке, похоже, что 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);
  }
              Ответить

      • ava ctm 14.03.2011 11:36 # +3

        в этом случае надо делать тогда так:
        code = request->GetResponseCode();
        if ((code >= 200) && (code < 300))
        но уж точно не делить на 100 и сравнивать с 2.
        Ответить

        • ava Lure Of Chaos 15.03.2011 00:20 # 0

          одно деление и одно сравнение против двух сравнений, коньюкции и еще одного неявного сравнения
          Ответить

          • ava rat4 15.03.2011 09:10 # +1

            > и еще одного неявного сравнения
            Что это? Где оно?
            Ответить

          • ava ctm 18.03.2011 11:43 # 0

            в скомпилированном варианте будет 2 последовательных сравнения с условными переходами.
            и это в любом случае лучше,чем деление + сравнение.
            Ответить

        • ava bugmenot 15.03.2011 09:26 # 0

          скобки лишние
          Ответить

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