0
\$\begingroup\$

I would like a review and recommendations for improvement of the code below. I want to allow an exception to be handled by the caller, so I am rethrowing it. But I have resources to close. So I have the resource closing code duplicated in both the try and the catch blocks. Is there a better way to do this?

const string HTTPopotamus::GET(void) {
  HINTERNET  hSession = NULL, 
      hConnect = NULL,
      hRequest = NULL;
    try {
        hSession = this->open();
        hConnect = this->connect(hSession);
        hRequest = this->openRequest(hConnect);
        this->sendRequest(hRequest);
        this->receiveResponse(hRequest);
        string strData = this->readData(hRequest);
        /* Close any open resources. */
        if( hRequest ) WinHttpCloseHandle( hRequest );
        if( hConnect ) WinHttpCloseHandle( hConnect );
        if( hSession ) WinHttpCloseHandle( hSession );
        return strData;
    } catch (const ExceptionCyclOps& e) {
        /* Close any open resources. */
        if( hRequest ) WinHttpCloseHandle( hRequest );
        if( hConnect ) WinHttpCloseHandle( hConnect );
        if( hSession ) WinHttpCloseHandle( hSession );
        throw;
    }

}
\$\endgroup\$
3
\$\begingroup\$

It seems that your code would benefit dramatically from use of RAII. To illustrate, let's assume the existence of an HTTPConnection class that opens an appropriate connection in its constructor and closes it at destruction. The code would look more like the following:

const string HTTPopotamus::GET(void) {
    HTTPConnection conn(/*connection parameters here*/);

    conn.sendRequest(/*request parameters here*/);
    conn.waitForResponse(HTTP_REQUEST_TIMEOUT);

    return this->readData(hRequest);
}

This approach is more elegant and clear and explains itself with no need for comments. Any exception thrown, such as exceptions that represent a failed connection or a timeout event would be propagated to the caller while the HTTPConnection object would still be destroyed properly.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.