std::string copy constructor fails

Discussion in 'C++' started by Simon, Dec 7, 2005.

  1. Simon

    Simon Guest

    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.
     
    Simon, Dec 7, 2005
    #1
    1. Advertising

  2. Simon

    Guest

    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
     
    , Dec 7, 2005
    #2
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Peter Jansson
    Replies:
    5
    Views:
    6,397
    Ivan Vecerina
    Mar 17, 2005
  2. Fei Liu
    Replies:
    9
    Views:
    467
  3. Jeffrey Walton
    Replies:
    10
    Views:
    966
    Mathias Gaunard
    Nov 26, 2006
  4. Replies:
    2
    Views:
    2,296
    John Harrison
    May 26, 2007
  5. cinsk
    Replies:
    35
    Views:
    2,695
    James Kanze
    Oct 11, 2010
Loading...

Share This Page