Re: is such exception handling approach good?

From:

=?Utf-8?B?R2Vvcmdl?= <George@discussions.microsoft.com>

Newsgroups:

microsoft.public.vc.language

Date:

Mon, 24 Dec 2007 05:51:00 -0800

Message-ID:

<8B780698-02B0-46CB-80E9-BE4AAFF86A8E@microsoft.com>

Hi ajk,

For your concern,

###
here if you get a memory exception in m_pConn you get a memory leak in
m_pBuff since the dtor will not be called. Since no object AllInOne
has been created you can't clean it up in your exception handler.
###

I think if we use auto_ptr for variable m_pBuff, then this issue could be
solved. Agree?

(if exception is thrown in OpenDB, the memory pointed by m_pBuff could be
freed.)

1. Establish defined state of the object in constructor:

AllInOne::AllInOne() :
m_pBuff(new BYTE[42]),
m_pConn(OpenDB(...))
m_fLog(fopen(...))
{
// do other stuff
}

Then you can reliably use its methods:

BYTE AllInOne::GetData(int i)
{
return m_pBuff[i];
}

void AllInOne::RetrieveData()
{
m_pConn->FillBuff(m_pBuff, 42);
}

regards,
George

"ajk" wrote:

On Sat, 22 Dec 2007 12:38:19 +0200, "Alex Blekhman"
<tkfx.REMOVE@yahoo.com> wrote:

It depends on the nature of a class. Sometimes I need none
of them in constructor; sometimes all of them.

The main purpose to do this in constructor is exactly in
order to avoid undefined state of the object. When you
construct it in phases, then complexity of class members
increases insanely. You just can't trust yourself inside
class memeber anymore and bound to perform tedious error
prone checks for everything. It harms both performance and
stability of the code, not to mention high maintenance
costs.

###
to get me wrong: I have nothing against initializing in the ctor just
I am against getting exception when initializing the object.

Suppose you have the class:

class AllInOne
{
   ...

private:
   BYTE* m_pBuf;
   DBConn* m_pConn;
   FILE* m_fLog;
};

Now you have two options:

1. Establish defined state of the object in constructor:

AllInOne::AllInOne() :
   m_pBuff(new BYTE[42]),
   m_pConn(OpenDB(...))
   m_fLog(fopen(...))
{
   // do other stuff
}

Then you can reliably use its methods:

BYTE AllInOne::GetData(int i)
{
   return m_pBuff[i];
}

void AllInOne::RetrieveData()
{
   m_pConn->FillBuff(m_pBuff, 42);
}

###
here if you get a memory exception in m_pConn you get a memory leak in
m_pBuff since the dtor will not be called. Since no object AllInOne
has been created you can't clean it up in your exception handler.

etc.

2. Create "simple" object first, then create necessary part
separately:

AllInOne::AllInOne() :
   m_pBuff(NULL),
   m_pConn(NULL)
   m_fLog(NULL)
{
}

AllInOne::InitBuff()
{
   if(!m_pBuff)
       m_pBuff = new BYTE[42];
   else
       assert("Cannot init the buffer twice!");
}

Now all of class' methods must paranoicaly check internal
state and hope for the best that user won't forget to call
relevant initializer functions:

###
true that there is some more housekeeping, but just normal
housekeeping that should have been done in your previous class as
well; memory gets overwritten, database connections get lost etc. you
need anyway to check these things

/ajk