I've read several questions and blog posts about flattening nested if
statements in general, but I was specifically wondering how to flatten nested if(SUCCEEDED(...))
statements when using the Windows API, when there's often some cleanup to be done like releasing pointers.
Here's some sample code from a blog post by Raymond Chen:
void CALLBACK RecalcText(HWND hwnd, UINT, UINT_PTR, DWORD)
{
HWND hwndFind = GetForegroundWindow();
g_szPath[0] = TEXT('\0');
g_szItem[0] = TEXT('\0');
IShellWindows *psw;
if (SUCCEEDED(CoCreateInstance(CLSID_ShellWindows, NULL, CLSCTX_ALL,
IID_IShellWindows, (void**)&psw))) {
VARIANT v;
V_VT(&v) = VT_I4;
IDispatch *pdisp;
BOOL fFound = FALSE;
for (V_I4(&v) = 0; !fFound && psw->Item(v, &pdisp) == S_OK;
V_I4(&v)++) {
IWebBrowserApp *pwba;
if (SUCCEEDED(pdisp->QueryInterface(IID_IWebBrowserApp, (void**)&pwba))) {
HWND hwndWBA;
if (SUCCEEDED(pwba->get_HWND((LONG_PTR*)&hwndWBA)) &&
hwndWBA == hwndFind) {
fFound = TRUE;
IServiceProvider *psp;
if (SUCCEEDED(pwba->QueryInterface(IID_IServiceProvider, (void**)&psp))) {
IShellBrowser *psb;
if (SUCCEEDED(psp->QueryService(SID_STopLevelBrowser,
IID_IShellBrowser, (void**)&psb))) {
IShellView *psv;
if (SUCCEEDED(psb->QueryActiveShellView(&psv))) {
IFolderView *pfv;
if (SUCCEEDED(psv->QueryInterface(IID_IFolderView,
(void**)&pfv))) {
IPersistFolder2 *ppf2;
if (SUCCEEDED(pfv->GetFolder(IID_IPersistFolder2,
(void**)&ppf2))) {
LPITEMIDLIST pidlFolder;
if (SUCCEEDED(ppf2->GetCurFolder(&pidlFolder))) {
if (!SHGetPathFromIDList(pidlFolder, g_szPath)) {
lstrcpyn(g_szPath, TEXT("<not a directory>"), MAX_PATH);
}
int iFocus;
if (SUCCEEDED(pfv->GetFocusedItem(&iFocus))) {
LPITEMIDLIST pidlItem;
if (SUCCEEDED(pfv->Item(iFocus, &pidlItem))) {
IShellFolder *psf;
if (SUCCEEDED(ppf2->QueryInterface(IID_IShellFolder,
(void**)&psf))) {
STRRET str;
if (SUCCEEDED(psf->GetDisplayNameOf(pidlItem,
SHGDN_INFOLDER,
&str))) {
StrRetToBuf(&str, pidlItem, g_szItem, MAX_PATH);
}
psf->Release();
}
CoTaskMemFree(pidlItem);
}
}
CoTaskMemFree(pidlFolder);
}
ppf2->Release();
}
pfv->Release();
}
psv->Release();
}
psb->Release();
}
psp->Release();
}
}
pwba->Release();
}
pdisp->Release();
}
psw->Release();
}
InvalidateRect(hwnd, NULL, TRUE);
}
Raymond notes that he'd probably refactor the code using subfunctions. Some of the comments to the post argue that goto
statements could be used. I've read that goto
should be avoided, but Microsoft seems to condone it in some sample code of its own.
If everything is taking place within a function, I'm wondering if you can just flip the conditionals and return with an early exit, making sure that the necessary memory is cleaned up in either case:
if (!SUCCEEDED(...)) {
whateverPointerWasBeingUsed->Release();
return SOME_SORT_OF_CODE;
}
whateverPointerWasBeingUsed->Release();
if(!SUCCEEDED(...)) {
whateverPointerWasBeingUsed2->Release();
return SOME_SORT_OF_CODE;
}
whateverPointerWasBeingUsed2->Release();
[and so on...]
But I am new to this, so maybe that poses some problems.
How should nested SUCCEEDED statements be refactored?
Aucun commentaire:
Enregistrer un commentaire