Нашел в рабочем проэкте

UINT CMainDlg::ThreadHendler_DownloadFile(LPVOID param)
{
    thesIsDownloadedFileComplete = false;
    ProgressDlg* progressDlgPtr = (ProgressDlg*)param;
    HRESULT hr;
    IUnknown* pUnk = NULL;
    DownloadStatusCallback* statusCallbackPtr = new DownloadStatusCallback;
    try
    {
        hr = statusCallbackPtr->QueryInterface(IID_IUnknown, (void**)&pUnk);
        if (FAILED(hr))
        {
            throw Exception(_T("Error, during getting IUnknown iFace"));
        }
        hr = pUnk->QueryInterface(IID_IBindStatusCallback, (void**)&statusCallbackPtr);
        if (FAILED(hr)) 
        {
            throw Exception(_T("Error, during getting IID_IBindStatusCallback iFace"));
        }       
        
        statusCallbackPtr->SetProgressDlgPtr(progressDlgPtr);
        if (FAILED(URLDownloadToFile(NULL, thesURL, thesVideoFilePath, 0, (IBindStatusCallback*)statusCallbackPtr))) 
        {
            throw long(-1);
        }
    }
    catch (Exception& e) 
    {
    progressDlgPtr->Cancel();
        ::MessageBox(0, e.ToString(), _T("Error."), MB_OK);
        statusCallbackPtr->Release();
        delete statusCallbackPtr;
        return -1;
    }
    catch (long e) 
    {
        if (e)
        {
        progressDlgPtr->Cancel();
        CString msg(_T("Downloading file is unavailable"));
        ::MessageBox(0, msg, _T("Error."), MB_OK | MB_ICONSTOP);
            statusCallbackPtr->Release();
            delete statusCallbackPtr;
            return -1;
        }
    }

    statusCallbackPtr->Release();
    delete statusCallbackPtr;
    thesIsDownloadedFileComplete = true;
    return 0;
//  [9/2/2004]
//  pUnk->Release();
//  delete pUnk;
    
//  statusCallbackPtr->Release();
//  delete statusCallbackPtr;

//  gIsDownloaded = true;
//    return 0;
}

Здравствуйте, j s p, Вы писали:

JSP>Нашел в рабочем проэкте

Тут не смеяться, тут плакать надо, причем над каждой строчкой.

Здравствуйте, algol, Вы писали:

A>Здравствуйте, j s p, Вы писали:
JSP>>Нашел в рабочем проэкте
A>Тут не смеяться, тут плакать надо, причем над каждой строчкой.

А можете пожалуйста прокомментировать над чем именно плакать?
Я заметил лишь:
— глупое использование флага = true/false
— не надо было заморачиваться с удалением вручную созданных объектов...
достаточно было воспользоваться умным указателем.
— Не совсем корректно продуманно... он бы уже либо кодами возврата пользовался,
либо уже исключениями...


Что еще упустил?

Здравствуйте, sjukov, Вы писали:

S>Здравствуйте, algol, Вы писали:

S>А можете пожалуйста прокомментировать над чем именно плакать?

S>Что еще упустил?

эквивалент:
UINT CMainDlg::ThreadHendler_DownloadFile(LPVOID param)
{
DownloadStatusCallback* statusCallbackPtr = new DownloadStatusCallback;
statusCallbackPtr->SetProgressDlgPtr((ProgressDlg*)param);
if (FAILED(URLDownloadToFile(NULL, thesURL, thesVideoFilePath, 0, (IBindStatusCallback*)statusCallbackPtr)))
{
progressDlgPtr->Cancel();
::MessageBox(0, _T("Downloading file is unavailable"), _T("Error."), MB_OK | MB_ICONSTOP);
}
statusCallbackPtr->Release();
delete statusCallbackPtr;
return 0;
}

многа букаф

Здравствуйте, sjukov, Вы писали:

S>Что еще упустил?

Видимо крайне оригинальный способ добычи интерфеса IBindStatusCallback
из класса-враппера DownloadStatusCallback:

1. DownloadStatusCallback* statusCallbackPtr= new DownloadStatusCallback;
   
2. statusCallbackPtr->QueryInterface(IID_IUnknown, (void**)&pUnk);
   
3. pUnk->QueryInterface(IID_IBindStatusCallback, (void**)&statusCallbackPtr);

Конечно, я не эксперт COM-программинга, возможно это в порядке вещей.
Правда, скорее всего случится меморилик, но судя по всему 9/2/2004 эту проблему пофиксили,
вероятно внедрением какого-то эвристического алгоритма подсчета ссылок в метод
~DownloadStatusCallback(). . Вобщем, деструктор в студию!
А так, есдинственная трабла, если вызов

pUnk->QueryInterface(IID_IBindStatusCallback, (void**)&statusCallbackPtr);

будет неудачен, первому catch'у придётся несладко.

З.Ы. Походу чел просто передознулся примерами от микрософта, всего делов-то

Здравствуйте, gerrCrazzy, Вы писали:

C>Здравствуйте, sjukov, Вы писали:

S>>Что еще упустил?

C>Видимо крайне оригинальный способ добычи интерфеса IBindStatusCallback
C>из класса-враппера DownloadStatusCallback:
C>

C>
1. DownloadStatusCallback* statusCallbackPtr= new DownloadStatusCallback;
   C>
2. statusCallbackPtr->QueryInterface(IID_IUnknown, (void**)&pUnk);
   C>
3. pUnk->QueryInterface(IID_IBindStatusCallback, (void**)&statusCallbackPtr);
   C>

C>Конечно, я не эксперт COM-программинга, возможно это в порядке вещей.
C>Правда, скорее всего случится меморилик, но судя по всему 9/2/2004 эту проблему пофиксили,
C>вероятно внедрением какого-то эвристического алгоритма подсчета ссылок в метод
C>~DownloadStatusCallback(). . Вобщем, деструктор в студию!

Способ супер-оригинальный учитывая, что DownloadStatusCallback наследник IBindStatusCallback.
Ликов никаких не будет так как в конце есть явный вызов оператора delete.
Ну а деструктор — он пустой.

Здравствуйте, j s p, Вы писали:

А можно мне позанудствовать? Спасибо.

Разве Release() не вызывает delete this; ?

JSP>эквивалент:
JSP>UINT CMainDlg::ThreadHendler_DownloadFile(LPVOID param)
JSP>{
JSP> DownloadStatusCallback* statusCallbackPtr = new DownloadStatusCallback;
JSP> statusCallbackPtr->SetProgressDlgPtr((ProgressDlg*)param);
JSP> if (FAILED(URLDownloadToFile(NULL, thesURL, thesVideoFilePath, 0, (IBindStatusCallback*)statusCallbackPtr)))
JSP> {
JSP> progressDlgPtr->Cancel();
JSP> ::MessageBox(0, _T("Downloading file is unavailable"), _T("Error."), MB_OK | MB_ICONSTOP);
JSP> }
JSP> statusCallbackPtr->Release();
JSP> delete statusCallbackPtr; // мне кажется, что это тоже лишнее.
JSP> return 0;
JSP>}

Здравствуйте, WoldemaR, Вы писали:

WR>Здравствуйте, j s p, Вы писали:

WR>А можно мне позанудствовать? Спасибо.

WR>Разве Release() не вызывает delete this; ?

ага, тут уже я запарился

Здравствуйте, sjukov, Вы писали:

S>А можете пожалуйста прокомментировать над чем именно плакать?

Ладно, поехали.
Судя по определению, это функция потока. Не самая лучшая идея определять ее как static в CMainDlg. В качестве параметра ей передается указатель на ProgressDlg (вероятно это все на MFC). Т.е. можно предположить, что ProgressDlg создан в другом потоке. Передавать указатели на оконные классы в другой поток нельзя, можно передавать только HWND.
Дальше идут просто зверские манипуляции вокруг DownloadStatusCallback. Раз уж этот класс так тесно завязан на ProgressDlg, логично было бы реализовать IBindStatusCallback в самом ProgressDlg. В любом случае DownloadStatusCallback наследован от IBindStatusCallback и вместо этих шаманских плясок можно было воспользоваться cast-ом. Когда делается QueryInterface, то нужно было указывать IBindStatusCallback*, а не сохранять его обратно в DownloadStatusCallback. Сам класс можно было бы создать на стеке и не заморачиваться с его удалением в catch.
Дальше мы имеет два catch на свои же генерируемые exceptions. Никаких других исключений тут быть не может. Всю обработку исключений можно было бы сразу запихнуть в if. За MessageBox-ы с нулевым HWND нужно просто сразу убивать.

Свои 5 копеек — если даже такой простой кусок кода
вызывает споры по поводу его поведения и безопасного удаления
всех выделенных ресурсов, то что же в остальной программе?

Здравствуйте, anidal, Вы писали:

A>Свои 5 копеек — если даже такой простой кусок кода
A>вызывает споры по поводу его поведения и безопасного удаления
A>всех выделенных ресурсов, то что же в остальной программе?

Да нет тут никакой проблемы. Товарищ *индус* просто забыл, что создает DownloadStatusCallback не через COM, а просто как экземпляр класса. Все эти Release() тут нафиг не нужны, нам по барабану, что будет в счетчике ссылок при удалении объекта. Кстати, это еще вопрос, что содержится в реализации AddRef/Release этого класса и подсчитываются ли ссылки реально.
А насчет остальной программы — если она написана в таком же стиле, то это просто хорошо упитанный пушной зверек.

Ведете себя как студенты.
Неужели не понятно, что нельзя ругать чужой код, если не знаешь как и в каких условиях это писалось. А мы поехали: "можно предоложить, что то, можно предположить, что се...".
Возможно человек ни разу в жизни не писал Си.. возможно, ему надо было за 10 минут исправить баг в чужой программе, да мало ли чего еще бывает в жизни.. может это не программист писал, а талантливый дворник, который один остался в развалившейся конторе. Его уважать надо, а не издеваться над ним.
В общем, ИМХО, надо быть мудрее.
Удачи.

Здравствуйте, sjukov, Вы писали:

S>Здравствуйте, algol, Вы писали:

A>>Здравствуйте, j s p, Вы писали:
JSP>>>Нашел в рабочем проэкте
A>>Тут не смеяться, тут плакать надо, причем над каждой строчкой.

S>А можете пожалуйста прокомментировать над чем именно плакать?
S>Я заметил лишь:
S>- глупое использование флага = true/false
S>- не надо было заморачиваться с удалением вручную созданных объектов...
S>достаточно было воспользоваться умным указателем.
S>- Не совсем корректно продуманно... он бы уже либо кодами возврата пользовался,
S> либо уже исключениями...


S>Что еще упустил?

Грамостность онглицкая хромает.

Роюсь в еще одном шедевре...

Стандартно встречаются такие вот конструкции

for(int j = 0; ... j++)
{
...
}
...
for(j = 0; ...)

Не указан возвращаемый тип метода в классах. Причем в реализациях метода иногда есть return 0, а иногда нет!

Но что меня совершенно убило:

BOOL CblablaApi::Init()
{
...
m_hDll = LoadLibrary(...);


#define BEGINSETFUCNTIONPOINTER(m)    { HMODULE hLib=m;int miss_func=0;
#define ENDSETFUCNTIONPOINTER(miss) miss=miss_func;}

    int miss=0;
    BEGINSETFUCNTIONPOINTER(m_hDll)

#define BLABLA_API_SET_FUNC_POINTER
    #include "BlaBlaApiDef.h"
#undef BLABLA_API_SET_FUNC_POINTER

    ENDSETFUCNTIONPOINTER(miss)
...
}

в файле BlaBlaApiDef.h тоже дохрена хитрых макросов.

Made In Corea! :crash: :maniac: