1

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.

Barmak Shemirani
  • 30,904
  • 6
  • 40
  • 77
Andrew Truckle
  • 17,769
  • 16
  • 66
  • 164
  • 1
    On an unrelated note, `CDatabase::Close()` does something else entirely. It implements the `try`/`finally` pattern that's common in languages like Java or C#. It allows marking a region of code that needs to run, regardless of whether an exception was thrown or not. Releasing locks is certainly something you'd always want to happen, and that's what the whole try-catch-unlock-rethrow dance is doing. If MFC were re-implemented today, it would just use a [scoped_lock](https://en.cppreference.com/w/cpp/thread/scoped_lock), and all the try-catch-rethrow noise would just magically disappear. – IInspectable Oct 24 '21 at 10:40

1 Answers1

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); }
Barmak Shemirani
  • 30,904
  • 6
  • 40
  • 77