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

    +160

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    12. 12
    13. 13
    14. 14
    15. 15
    16. 16
    17. 17
    18. 18
    19. 19
    20. 20
    21. 21
    22. 22
    23. 23
    24. 24
    25. 25
    26. 26
    27. 27
    28. 28
    29. 29
    30. 30
    31. 31
    32. 32
    33. 33
    34. 34
    35. 35
    36. 36
    37. 37
    38. 38
    39. 39
    40. 40
    41. 41
    42. 42
    43. 43
    44. 44
    45. 45
    46. 46
    47. 47
    48. 48
    49. 49
    50. 50
    51. 51
    52. 52
    53. 53
    54. 54
    55. 55
    56. 56
    57. 57
    58. 58
    59. 59
    60. 60
    61. 61
    62. 62
    63. 63
    64. 64
    65. 65
    66. 66
    67. 67
    68. 68
    69. 69
    70. 70
    71. 71
    72. 72
    73. 73
    74. 74
    75. 75
    76. 76
    77. 77
    78. 78
    79. 79
    80. 80
    81. 81
    HRESULT GetOutlookVersionString(LPSTR* ppszVer, BOOL* pf64Bit)
    {
        HRESULT hr = E_FAIL;
        LPSTR pszTempPath = NULL;
        LPSTR pszTempVer = NULL;
        TCHAR pszaOutlookQualifiedComponents[][MAX_PATH] = {
            TEXT("{1E77DE88-BCAB-4C37-B9E5-073AF52DFD7A}"), // Outlook 2010
            TEXT("{24AAE126-0911-478F-A019-07B875EB9996}"), // Outlook 2007
            TEXT("{BC174BAD-2F53-4855-A1D5-0D575C19B1EA}")  // Outlook 2003
        };
    
        int nOutlookQualifiedComponents = _countof(pszaOutlookQualifiedComponents);
        int i = 0;
        DWORD dwValueBuf = 0;
        UINT ret = 0;
    
        *pf64Bit = FALSE;
    
        for (i = 0; i < nOutlookQualifiedComponents; i++)
        {
            ret = MsiProvideQualifiedComponent(
                pszaOutlookQualifiedComponents[i],
                TEXT("outlook.x64.exe"),
                (DWORD) INSTALLMODE_DEFAULT,
                NULL,
                &dwValueBuf);
            if (ERROR_SUCCESS == ret) break;
        }
    
        if (ret != ERROR_SUCCESS)
        {
            ret = MsiProvideQualifiedComponent(
                pszaOutlookQualifiedComponents[i],
                TEXT("outlook.exe"),
                (DWORD) INSTALLMODE_DEFAULT,
                NULL,
                &dwValueBuf);
        }
        else
        {
            *pf64Bit = TRUE;
        }
    
        if (ret == ERROR_SUCCESS)
        {
            dwValueBuf += 1;
            pszTempPath = (LPSTR) malloc(dwValueBuf * sizeof(TCHAR));
    
            if (pszTempPath != NULL)
            {
                if ((ret = MsiProvideQualifiedComponent(
                    pszaOutlookQualifiedComponents[i],
                    TEXT("outlook.exe"),
                    (DWORD) INSTALLMODE_EXISTING,
                    pszTempPath,
                    &dwValueBuf)) != ERROR_SUCCESS)
                {
                    goto Error;
                }
    
                pszTempVer = (LPSTR) malloc(MAX_PATH * sizeof(TCHAR));
                dwValueBuf = MAX_PATH;
                if ((ret = MsiGetFileVersion(pszTempPath,
                    pszTempVer,
                    &dwValueBuf,
                    NULL,
                    NULL))!= ERROR_SUCCESS)
                {
                    goto Error;    
                }
                *ppszVer = pszTempVer;
                pszTempVer = NULL;
                hr = S_OK;
            }
        }
    
    Error:
        free(pszTempVer);
        free(pszTempPath);
        return hr;
    }

    Говнокод от САМОГО Билли... 21 век на дворе, а у нас в C++ коде goto Error написано (точнее - накакано).
    Источник - http://msdn.microsoft.com/en-us/library/dd941331.aspx

    Запостил: kVolt, 09 Сентября 2011

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

    • еще одна жертва "не за что не используйте goto в своих программах!"?
      Ответить
      • нет. жертва постоянного применения исключения из этого правила.
        Ответить
        • Это как раз нормальное использование гото.
          Ответить
          • +1
            В MS коде везде так ошибки обрабатываются, это стало идиомой. Кроме того, такое использование goto приветствует Макконнелл: меньше дублирования кода.
            Ответить
            • "goto Error;" продублировано два раза.
              Ответить
              • да ты чё
                Ответить
                • throwExceptionByHResult(hr);
                  По моему это лучше, чем
                  if (ret == ERROR_SUCCESS)
                  {
                    obj1->Release();
                    ...
                    onjn->Release();
                    goto err;
                  };
                  И так после вызова каждой функции.
                  Ответить
                  • Вообще-то Release() находится после err
                    Ответить
                    • Это очень глупо, когда:
                      IUnknown *a, *b;
                      HRESULT r=makeComObject(&a,  guid1);
                      if(r!=ERROR_SUCCESS)
                        goto err;
                      r=makeComObject(&b,  guid2);
                      if(r!=ERROR_SUCCESS)
                        goto err;
                      ...
                      err:
                        a->Release();
                        b->Release();
                      Особенно с учетом того, что b ещё может быть не создан, когда мы входим в err. Поэтому, с такой практикой err приходится удалять все объекты вручную внутри if'а перед переходом goto err;.

                      Есть, конечно, реализации SafeRelease, но это костыль.

                      Гуглите реализацию ComPtr<IТипИнтерфейс> comObj.
                      Этот частный случай смартпоинтера сам вызовет Release, даже если неожиданно произойдет исключение. Да и работать с ним удобнее, чем с голым интерфейсом.
                      Ответить
                      • > Этот частный случай смартпоинтера сам вызовет Release

                        Я понимаю, что на РАИИ всё это делается вообще без усилий и само по себе.
                        Но если говорить про низкий уровень, то гото - лучший вариант. Ну и отдельно флаги, хранящие, кого мы созали, а кого-нет.
                        Ответить
                        • Вы про Си или про неправильное использование С++, за которое он иногда бьет по рукам?
                          Ответить
                          • Микрософт принипиально пишет системные функции без крестофишечек (и это правильно), так что будем говорить про Си.
                            Ответить
                            • Но использовать то системные функции можно и в С++. Си интерфейс системных библиотек не накладывает ограничений на код их использующий.

                              Даже системные функции можно на С++ писать, главное наружу выдавать Си интерфейс и не выпускать исключений.
                              Ответить
                              • > главное наружу выдавать Си интерфейс и не выпускать исключений

                                Это проще, если сразу всё на чистом Си писать.
                                Ответить
                                • На чистом Си никогда не было проще. В стандартной библиотеке Си даже высокоуровневых структур и примитивов нет. Приходится все вручную велосипедить.
                                  Ответить
                            • Да у МС даже компилятора Си нет.
                              Ответить
            • >Макконнелл
              Странно, с такой фамилией и против дублирования.
              Ответить
              • Если бы он был за дублирование, то он был бы
                Маконелмаконел
                А так он провёл оптимизацию и сделал так, что дублируются только три буквы.
                Ответить
              • не странно, ему и этого хватило ))
                Ответить
              • >Странно, с такой фамилией и против дублирования.
                Ему пришлось иметь дело с наследованием старого кода
                Ответить
      • Не, это жертва копи-пейста. В последнем условии присваивание не нужно, но оно есть, потому что оно есть в 4 предыдущих условиях...
        Ответить
    • ГДЕ ЗДЕСЬ C++?
      Ответить
      • ответ на вопрос - по пруфлинку ) Судя по всему, MS искренне считает, так должен выглядеть тру-C++ код )
        Ответить
    • Это Си. Хотя все равно говно
      Ответить
    • ERROR_SUCCESS - диалектический дуализм.
      Всегда доставляло число аргументов в API -функциях мелкомягких.
      Ответить
    • > HRESULT hr = E_FAIL;

      E от слова Epic?
      Ответить
      • От слова Error, но признаю, странное идентификаторообразование у ms...
        Ответить
    • Идентификация Билли
      Ответить
    • >Говнокод от САМОГО Билли...
      тоже мне новость.
      Ответить
    • > Говнокод от САМОГО Билли
      Да ладно, всем понятно, что это написали индусы за чизбургер и баночку колы
      Ответить
    • > ERROR_SUCCESS
      кажется, это эпидемия
      Ответить
    • Билли Токарев, мля
      Ответить
    • показать все, что скрытоХм, а Билли программистом стал, очень весело. Хм, когда это Билли что-то программил?
      Ответить
    • Здесь прекрасно всё. Захардкоженный кабаллистический массив pszaOutlookQualifiedComponents. Гарантированный выход за его пределы в 33. Повторная проверка в 30 и 44. Ну и спагетти-код.
      Ответить
      • > Повторная проверка в 30 и 44
        ret между этими вызовами мог измениться (строка 32)

        в 30-42 вообще какой-то трэш происходит. Почему x64 версия проверяется для всех ключей, а x86 - не для всех?
        Ответить
        • Только в одной из веток после 30! Почему бы не засунуть другой if прямо туда? Вообще, если грамотно расписать структурно, так может никакие goto и не понадобились бы, и код стал бы читаться яснее.
          Ответить
      • И венгерская нотация, но MS вообще это любят.
        Ответить
        • вы так говорите, буд-то это что-то плохое
          Ответить
          • Вы так говорите, как будто это что-то хорошее
            Ответить
            • а че плохого-то? вреда ноль, привыкания ноль...
              Ответить
              • Это мусолят во всех приличных книгах по разработке ПО, даже не хочется снова приводить эти аргументы.
                Ответить
    • Код нужно осовременить. Переписать с использованием try...catch.
      Ответить
      • Это ж надо сто тыщ индусов переучивать... Не, затратно.
        Ответить
        • тем более, что на этот костыль где то свой костыль прибит....нее....сцыкотно...вдруг сломается.... а так сразу видно, что если еррор - то иди на еррор, и никаких сусликов!
          Ответить
      • В такой низкоуровневый код исключения не влазят.
        Ответить
        • Просто нужно возвращать строку, а не какой-то HRESULT, и сразу появится место для исключений.
          Ответить
          • Что возвращать? Указатель на выделенную функцией область памяти?
            Это системная функция, её вызывают из разных языков, поэтому нельзя в её сигнатуре писать ничего сложнее указателей и целых чисел, иначе с форматом объектов будет путаница.
            Ответить
      • >Переписать с использованием try...catch
        Лучше с использованием RAII. Не путать с Red Alert II Это более удобно, безопасно и менее многословно.
        Ответить
    • Тред не читал. Код - говно. goto для быдла.

      Кто-нибудь сказал, об использовании?
      do{
      ...
         if(...){ 
            ...
            break;
        }
      }
      while(0==1)

      так структурней.
      Ответить
      • > while(0==1)
        while(((false==true)!=true ? 0 : 1) > 0)
        Ответить
      • ага, теперь вспоминаем, что у нас 3 вложенных цикла стало
        Ответить
    • Тут используются старые интерфейсы виндовса типа HRESULT.
      Как-то эстетически не очень приятно мешать коды возврата с исключениями.
      Ответить
    • показать все, что скрытоvanished
      Ответить

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