Re: Does this look scary to you?
Leo V wrote:
Okay, I've rewritten the function to hopefully be cleaner.
Please give me your thoughts.
STDMETHODIMP CForm::GetStringValue(LONG lFieldID, /* [out,retval] */
BSTR* pBstr)
{
if (!pBstr)
return E_POINTER;
_ASSERTE(!(*pBstr) && _T("This should be NULL! It's either not
initialized or is already pointing to allocated memory!"));
At this point I would add *pBstr = NULL, so even lazy clients which hand you
uninitialized BSTRs (which is OK for out, retval!) get a valid *pBstr value
back
CString csDesc = GetRuntimeText(lFieldID);
if (csDesc.IsEmpty())
return E_FAIL; // Is this really a failure?
That depends on you contract definition. You might consider E_INVALIDARG to
indicate that lFieldID was wrong as it doesn't exist.
try
{
*pBstr = csDesc.AllocSysString();
}
catch(CMemoryException* pme)
{
// NEVER throw an exception over a COM boundary.
*pBstr = NULL;
pme->Delete();
} // catch (CMemoryException*)
catch(...)
I am not a fan (any more) of swallowing ... exceptions. Those exceptions
might be the cause of a bug and it is best if the app crashes as soon as
possible, so you are close to the bug which might make finding it easier. In
some server components I tend to write an application log entry and then
rethrow the error - while that is not always possible if the bug has
overwritten enough process memory. So sometimes I have a trace where I first
caught the exception and can start to check later on where it might have
come from.
{
// NEVER throw an exception over a COM boundary.
*pBstr = NULL;
} // catch everything else
if (*pBstr == NULL)
return E_OUTOFMEMORY;
return S_OK;
}
--
SvenC