std::string copy constructor fails

[Simon]

Hi,

I have a class that does not seem to work.
I cannot see the problem, and the "fix" I have found does not help me
understand what the problem was

I know I don't need a copy constructor but the class might grow later and I
prefer having my copy constructor.
This also explains why I have 2 similar function "ClearAll() and NullAll()"
they might be used later in case I have pointers.

I use the class in a vector declared
std::vector< CMyClass > m_vMyClass;
and all I do is push_back(...), erase(...) and operator[...].

// this is the original class.
class CMyClass
{
private:
std::string m_sID;
std::string m_sName;
public:
void NullAll()
{
m_sID = ""; //<== Problem 1
m_sName = ""; //<== Problem 2
}

void ClearAll()
{
m_sID = ""; //<== Problem 3
m_sName = ""; //<== Problem 4
}

~CMyClass()
{
ClearAll();
}
CMyClass()
{
NullAll();
}
CMyClass( const CMyClass&mc )
{
NullAll();
*this = mc;
}
const CMyClass& operator=(const CMyClass&mc)
{
if( this != &mc )
{
NullAll();
ClearAll();

m_sID = mc.m_sID; //<== Problem 5
m_sName = mc.m_sName; //<== Problem 6
}
return *this;
}

}

///////////////////////////////////////////////////

All the problems where causing some sort of memory assertions.

My 'Fixes' for problem 1, 2, 3 and 4 were
m_sID ="" to
m_sID.erase() and

m_sName ="" to
m_sName.erase();

And the fix for problem 5 and 6 were
m_sID = mc.m_sID to
m_sID = std::string( mc.m_sID.c_str() ); and

m_sName = mc.m_sName to
m_sName = std::string( mc.m_sName.c_str() );

It works but I am not sure why the original code was causing a memory error.

Many thanks in advance.

Simon.

[Michiel.Salters]

Simon wrote:

Hi,

I have a class that does not seem to work.

I know I don't need a copy constructor but the class might grow later and I
prefer having my copy constructor.
This also explains why I have 2 similar function "ClearAll() and NullAll()"
they might be used later in case I have pointers.
That's probably a bad idea. You shouldn't think about pointer members
being 0
in a high-level class. More likely, you should think about optional
members
being not present. Also, it you use smart pointers for your members,
you get
these functions for free on the smart pointer class.
I use the class in a vector declared
std::vector< CMyClass > m_vMyClass;
and all I do is push_back(...), erase(...) and operator[...].
Ok - that seems like it can only fail if erasing an invalid iterator,
or
indexing out of bounds. Both are likely to corrupt memory.
// this is the original class.
class CMyClass
{
private:
std::string m_sID;
std::string m_sName;
public:
void NullAll()
{
m_sID = ""; //<== Problem 1
m_sName = ""; //<== Problem 2
} ....
All the problems where causing some sort of memory assertions.

Not because of those lines, but because your memory check mechanism
detected earlier corruption at that point. Fixes to those lines just
hide the bug.

HTH,
Michiel Salters