这种使用goto的代码怎么改比较优雅？这算糟糕的代码风格吗？

[善良超锅锅]

HRESULT AddSourceNode(
    IMFTopology *pTopology,           // Topology.
    IMFMediaSource *pSource,          // Media source.
    IMFPresentationDescriptor *pPD,   // Presentation descriptor.
    IMFStreamDescriptor *pSD,         // Stream descriptor.
    IMFTopologyNode **ppNode)         // Receives the node pointer.
{
    IMFTopologyNode *pNode = NULL;

    // Create the node.
    HRESULT hr = MFCreateTopologyNode(MF_TOPOLOGY_SOURCESTREAM_NODE, &pNode);
    if (FAILED(hr))
    {
        goto done;
    }

    // Set the attributes.
    hr = pNode->SetUnknown(MF_TOPONODE_SOURCE, pSource);
    if (FAILED(hr))
    {
        goto done;
    }

    hr = pNode->SetUnknown(MF_TOPONODE_PRESENTATION_DESCRIPTOR, pPD);
    if (FAILED(hr))
    {
        goto done;
    }

    hr = pNode->SetUnknown(MF_TOPONODE_STREAM_DESCRIPTOR, pSD);
    if (FAILED(hr))
    {
        goto done;
    }
    
    // Add the node to the topology.
    hr = pTopology->AddNode(pNode);
    if (FAILED(hr))
    {
        goto done;
    }

    // Return the pointer to the caller.
    *ppNode = pNode;
    (*ppNode)->AddRef();

done:
    SafeRelease(&pNode);
    return hr;
}

每一步都要判断有么有成功。不成功的操作都是一样的，怎么改呢？

我想到用一种局部变量来管理清理代码，把要执行的清理操作放在局部变量的析构函数中。在函数开头就定义一个局部变量，反正函数退出时，局部变量的析构函数是会自动调用的。但是这样无缘无故多了个局部变量，而且定义后没对它执行任何操作，会不会让人看不懂？

[dustpg]

goto用在这里有点糟糕，用SUCCEEDED就行了，你这里还不如用COM智能指针然后错误直接范围。 用FAILED就要保持错误信息，不保存用SUCCEEDED或者COM智能指针直接返回。 全用SUCCEEDED很好看:

        // 创建直接组合(Direct Composition)设备
        if (SUCCEEDED(hr)) {
            hr = LongUI::Dll::DCompositionCreateDevice(
                UIManager_DXGIDevice,
                LongUI_IID_PV_ARGS(m_pDcompDevice)
            );
            longui_debug_hr(hr, L"DCompositionCreateDevice faild");
        }
        // 创建直接组合(Direct Composition)目标
        if (SUCCEEDED(hr)) {
            hr = m_pDcompDevice->CreateTargetForHwnd(
                m_hwnd, true, &m_pDcompTarget
            );
            longui_debug_hr(hr, L"m_pDcompDevice->CreateTargetForHwnd faild");
        }
        // 创建直接组合(Direct Composition)视觉
        if (SUCCEEDED(hr)) {
            hr = m_pDcompDevice->CreateVisual(&m_pDcompVisual);
            longui_debug_hr(hr, L"m_pDcompDevice->CreateVisual faild");
        }
        // 设置当前交换链为视觉内容
        if (SUCCEEDED(hr)) {
            hr = m_pDcompVisual->SetContent(m_pSwapChain);
            longui_debug_hr(hr, L"m_pDcompVisual->SetContent faild");
        }
        // 设置当前视觉为窗口目标
        if (SUCCEEDED(hr)) {
            hr = m_pDcompTarget->SetRoot(m_pDcompVisual);
            longui_debug_hr(hr, L"m_pDcompTarget->SetRoot faild");
        }
        // 向系统提交
        if (SUCCEEDED(hr)) {
            hr = m_pDcompDevice->Commit();
            longui_debug_hr(hr, L"m_pDcompDevice->Commit faild");
        }

[善良超锅锅]

引用 1 楼 dustpg 的回复:

goto用在这里有点糟糕，用SUCCEEDED就行了，你这里还不如用COM智能指针然后错误直接范围。 用FAILED就要保持错误信息，不保存用SUCCEEDED或者COM智能指针直接返回。 全用SUCCEEDED很好看:

        // 创建直接组合(Direct Composition)设备
        if (SUCCEEDED(hr)) {
            hr = LongUI::Dll::DCompositionCreateDevice(
                UIManager_DXGIDevice,
                LongUI_IID_PV_ARGS(m_pDcompDevice)
            );
            longui_debug_hr(hr, L"DCompositionCreateDevice faild");
        }
        // 创建直接组合(Direct Composition)目标
        if (SUCCEEDED(hr)) {
            hr = m_pDcompDevice->CreateTargetForHwnd(
                m_hwnd, true, &m_pDcompTarget
            );
            longui_debug_hr(hr, L"m_pDcompDevice->CreateTargetForHwnd faild");
        }
        // 创建直接组合(Direct Composition)视觉
        if (SUCCEEDED(hr)) {
            hr = m_pDcompDevice->CreateVisual(&m_pDcompVisual);
            longui_debug_hr(hr, L"m_pDcompDevice->CreateVisual faild");
        }
        // 设置当前交换链为视觉内容
        if (SUCCEEDED(hr)) {
            hr = m_pDcompVisual->SetContent(m_pSwapChain);
            longui_debug_hr(hr, L"m_pDcompVisual->SetContent faild");
        }
        // 设置当前视觉为窗口目标
        if (SUCCEEDED(hr)) {
            hr = m_pDcompTarget->SetRoot(m_pDcompVisual);
            longui_debug_hr(hr, L"m_pDcompTarget->SetRoot faild");
        }
        // 向系统提交
        if (SUCCEEDED(hr)) {
            hr = m_pDcompDevice->Commit();
            longui_debug_hr(hr, L"m_pDcompDevice->Commit faild");
        }

那改成都用SUCCEEDED还需要调用SafeRelease(&pNode)吗？

[小灸舞]

goto的使用, 一个是跳出多层循环时使用, 还有一个是在性能考虑. 其他场景下基本上没有goto什么事, 特别在现代C++的情况下. 我的个人结论是除非是非goto不可的情况下绝不使用goto. 喜欢用goto说明姿势水平还不够高, 没有找到能很好描述逻辑的方式和工具, 还需要学习

[dustpg]

SUCCEEDED只是逻辑变了，该释放的还是要啊，COM智能指针就省略咯。 goto要用到刀刃上，用了goto逻辑简单了,又方便维护，肯定要用。 这里逻辑没有全用SUCCEEDED好，也没有goto语句需要额外维护，当然不要用. goto C++用的太少了，主要原因是异常以及跳过对象初始化， C用的还行，因为没有异常，也没有构造函数.

[chehw_1]

没必要改。这是一种很正常的C风格。

[ztenv]

挺糟糕的，不建议使用； 使用一次循环就能解决这个问题

[善良超锅锅]

引用 9 楼 akirya 的回复:

为啥不用智能指针？COM有好几种智能指针的。 使用局部变量清理是正确的做法。

COM心理负担好重。毕竟COM比较难

[yshuise]

写词法分析器的时候用得着goto

[赵4老师]

HRESULT AddSourceNode(
    IMFTopology               *pTopology, // Topology.
    IMFMediaSource            *pSource  , // Media source.
    IMFPresentationDescriptor *pPD      , // Presentation descriptor.
    IMFStreamDescriptor       *pSD      , // Stream descriptor.
    IMFTopologyNode          **ppNode   ) // Receives the node pointer.
{
    IMFTopologyNode *pNode = NULL;
    while (1) {
        // Create the node.
        HRESULT hr = MFCreateTopologyNode(MF_TOPOLOGY_SOURCESTREAM_NODE, &pNode); if (FAILED(hr)) break;
        // Set the attributes.
        hr = pNode->SetUnknown(MF_TOPONODE_SOURCE, pSource);                      if (FAILED(hr)) break;
        hr = pNode->SetUnknown(MF_TOPONODE_PRESENTATION_DESCRIPTOR, pPD);         if (FAILED(hr)) break;
        hr = pNode->SetUnknown(MF_TOPONODE_STREAM_DESCRIPTOR, pSD);               if (FAILED(hr)) break;
        // Add the node to the topology.
        hr = pTopology->AddNode(pNode);                                           if (FAILED(hr)) break;
        // Return the pointer to the caller.
        *ppNode = pNode;
        (*ppNode)->AddRef();
    }
    SafeRelease(&pNode);
    return hr;
}

[paschen]

你想写成这样也不算糟糕，MFC中也挺多类似这样的代码的

[珍惜生命远离CPP]

为啥不用智能指针？COM有好几种智能指针的。 使用局部变量清理是正确的做法。

[trytry1992]

正常风格啊，只要能一目了然就不是糟糕的风格