Rule of three in C++ programming

Question

I'm implementing a trie class, and following the rule of three in C++: has both nondefault coppy constructor, assignment operator and the destructor. However the program still crashes. What's wrong with the code?

class Trie
{
private:
    char ch;
    std::vector<Trie*> children;

public:
    std::string* word;
    static std::size_t nodeCount;
    Trie()
    {
        ch=0;
        word=NULL;
        ++nodeCount;
    };

    Trie(char c)
    {
        ch=c;
        word=NULL;
        ++nodeCount;
    };

    ~Trie()
    {
        cleanup();
        --nodeCount;
    };

    Trie(const Trie& source)
    {
        std::cout<<"hello world!\n";
        ch=source.ch;       
        if (source.word)
            word = new std::string(*(source.word));
        for (int i=0; i<int(source.children.size()); i++)
        {
            Trie* newA=new Trie(*(source.children[i]));
            children.push_back(newA);
        }
    }

    Trie& operator=(const Trie &source)
    {
        cleanup();
        ch=source.ch;       
        if (source.word)
            word = new std::string(*(source.word));
        for (int i=0; i<int(source.children.size()); i++)
        {
            Trie* newA=new Trie(*(source.children[i]));
            children.push_back(newA);
        }
        return *this;
    }
    void cleanup()
    {
        for (int i=0; i<int(children.size()); i++)
        {
            delete children[i];
        }
        if (word)
            delete word;
    }

P/S: This is the test function for deconstructor and the code pass the test:

TEST(memory)
{
  std::size_t base = Trie::nodeCount;
  // many nodes in the global Boggle trie
  CHECK( base == 0 );
  Trie *trie = new Trie();
  CHECK_EQUAL( base + 1, Trie::nodeCount );
  trie->addWord( "and" );
  CHECK_EQUAL( base + 4, Trie::nodeCount );
  trie->addWord( "ant" );
  CHECK_EQUAL( base + 5, Trie::nodeCount );
  delete trie;
  CHECK_EQUAL( base, Trie::nodeCount );
}

However the code failed the copy constructor and assignment test:

TEST(copyTrie)
{
  Trie trie;
  trie.addWord( "and" );
  trie.addWord( "ant" );
  CHECK_EQUAL( "and", *(trie.next('a')->next('n')->next('d')->word) );
  CHECK_EQUAL( "ant", *(trie.next('a')->next('n')->next('t')->word) );

  Trie copy( trie );
  CHECK_EQUAL( "and", *(copy.next('a')->next('n')->next('d')->word) );
  CHECK_EQUAL( "ant", *(copy.next('a')->next('n')->next('t')->word) );

  copy.addWord( "bee" );
  trie.addWord( "cat" );
}

It crashed exactly at the last command trie.addWord("cat")

possible duplicate of [What is The Rule of Three?](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — πάντα ῥεῖ, Dec 20 '13 at 00:03
I suggest using the copy-swap idiom if you're implementing `operator=`. Better yet would be no manual memory management, but that's unlikely here. — chris, Dec 20 '13 at 00:04
How is it duplicate ? if he asks about specific problem in his code ? — Coldsteel48, Dec 20 '13 at 00:04
could you debug it and tell us where does it crash , cuz I feel like looking for a match inside a a forest. — Coldsteel48, Dec 20 '13 at 00:06
It crashes when I call copy constructor Trie testcopy(trie). It traverses to all leaf nodes then crashes. — Dzung Nguyen, Dec 20 '13 at 00:07
Why do you have a `std::string*` member? Why not just `std::string`? And there's no need to check for `nullptr` before `delete`ing. — Praetorian, Dec 20 '13 at 00:08
As Joe Gauterin said, the problem is that your are cleaning the pointer, but not the container. So, in copy constructor, you are using trash contents yet. Try to put, at the end of cleanup function, the following statement: `children.clear()` — Amadeus, Dec 20 '13 at 00:19
"Rule of Three" is good. ["Rule of zero" is better](http://flamingdangerzone.com/cxx11/2012/08/15/rule-of-zero.html). — Mooing Duck, Dec 20 '13 at 00:56

score 2 · Answer 1 · answered Dec 20 '13 at 00:01

2

Your cleanup function doesn't empty the children vector.

answered Dec 20 '13 at 00:01

JoeG

12,994
1
38
63

1

Then what is `cleanup` doing? – David G Dec 20 '13 at 00:02
1

I'm really not sure, but I had a loop that delete everything in children vector. Is that the correct way? – Dzung Nguyen Dec 20 '13 at 00:04
3

@0x499602D2: No, it isn't. Dzung Nguyen: You are deleting the pointed at objects but not removing the pointers. – JoeG Dec 20 '13 at 00:06
4

@JoeGauterin Your answer can easily be misinterpreted (although you're correct). You could emphasize the difference between deleting elements and clearing the container itself, also why it's crucial in this case to do the latter. – leemes Dec 20 '13 at 00:08

Rule of three in C++ programming

1 Answers1