Correctly catching the CDatabase::Close exception

Question

I thought I would do a little digging about cataching exceptions.

According to this question (C++ catching all exceptions) one of the answers states:

[catch(...)] will catch all C++ exceptions, but it should be considered bad design.

---

At the moment I have used this approach:

CPTSDatabase::~CPTSDatabase()
{
    try
    {
        CloseDatabase();
    }
    catch(...)
    {
    }
}

void CPTSDatabase::CloseDatabase()
{
    if (m_dbDatabase.IsOpen())
        m_dbDatabase.Close();
}

I thought that this was the correct way because when I trace into CDatabase::Close() it does something similar:

// Disconnect connection
void CDatabase::Close()
{
    ASSERT_VALID(this);

    // Close any open recordsets
    AfxLockGlobals(CRIT_ODBC);
    TRY
    {
        while (!m_listRecordsets.IsEmpty())
        {
            CRecordset* pSet = (CRecordset*)m_listRecordsets.GetHead();
            pSet->Close();  // will implicitly remove from list
            pSet->m_pDatabase = NULL;
        }
    }
    CATCH_ALL(e)
    {
        AfxUnlockGlobals(CRIT_ODBC);
        THROW_LAST();
    }
    END_CATCH_ALL
    AfxUnlockGlobals(CRIT_ODBC);

    if (m_hdbc != SQL_NULL_HDBC)
    {
        RETCODE nRetCode;
        AFX_SQL_SYNC(::SQLDisconnect(m_hdbc));
        AFX_SQL_SYNC(::SQLFreeConnect(m_hdbc));
        m_hdbc = SQL_NULL_HDBC;

        _AFX_DB_STATE* pDbState = _afxDbState;

        AfxLockGlobals(CRIT_ODBC);
        ASSERT(pDbState->m_nAllocatedConnections != 0);
        pDbState->m_nAllocatedConnections--;
        AfxUnlockGlobals(CRIT_ODBC);
    }
}

And the CDatabase::Close documentation does not even state anything about exceptions being thrown.

---

The linked answer does state:

You can use c++11's new current_exception mechanism.

It is not clear if we can use this approach given the CDatabase class we are using.

Answer 1

Since CDatabase::Close() is using THROW_LAST to throw CDBException, you have to use catch (CDBException* e). Even if you are not handling it, you still have to Delete the error. You might as well do this when CDatabase methods are called directly:

void CPTSDatabase::CloseDatabase()
{
    try
    {
        if (m_dbDatabase.IsOpen())
            m_dbDatabase.Close();
    }
    catch (CDBException* e) 
    { 
        //TRACE(L"DB error: " + e->m_strError); 
        e->Delete(); 
    }
}

Or use

CPTSDatabase::~CPTSDatabase()
{
    try { CloseDatabase(); }
    catch (CDBException* e) { e->Delete();  }
    catch(...) {}
}

Because in this code it's not clear where the exceptions are coming from. catch(...) {} will deal with other exceptions. In general catch(...) {} is not recommended because it doesn't give useful information, it just says "something went wrong..."

Use Standard Library exceptions only if you are adding throw in your own code, or when using std functions. Example:

try { std::stoi("wrong argument"); }
catch (const std::exception& e) { TRACE("%s\n", e.what()); }

try { throw 123; }
catch (int i) { TRACE("%d\n", i); }