help with insert on first BST

Question

EDIT there a small thing that I am missing!! the error is still there

So I am attempting to learn how to code my first BST, and it is hard.... I am already having trouble with just a few lines of codes. the problem is in the insert, but I have included everything so that I could get some feedback on my style/other errors. I was suggested to use a pointer to pointer implementation, but we havent learned it yet, so I dont feel comfort/know how to code it yet. the

error is

[trinhc@cs1 Assignment_3]$ g++ movieList.cpp -o a.out
/tmp/ccLw6nsv.o: In function `main':
movieList.cpp:(.text+0x7a): undefined reference to `Tree::Tree()'
movieList.cpp:(.text+0xa7): undefined reference to `Tree::insert(int, std::basic_string<char, std::char_traits<char>, std::allocator<char> >)'
collect2: ld returned 1 exit status

the tree.h file

#ifndef TREE_H
#define TREE_H

#include <string>
#include <iostream>
using namespace std;

class Tree
{
 public:
  Tree();
  bool insert(int k, string s);

 private:
  struct Node
  {
    int key;
    string data;
    Node *left;
    Node *right;
  };
  Node* root;
  bool insert(Node*& root, int k, string s);
};

#endif

tree.cpp

#include <iostream>
#include "tree.h"
#include <stack>
#include <queue>
#include <string>
using namespace std;

Tree::Tree()
{
  root = NULL;
}

bool Tree::insert(int k, string s)
{
  return insert(root, k, s);
}

bool Tree::insert(Node*& current_root, int k, string s)
{
  if(root == NULL){
    current_root = new Node;
    current_root->key = k;
    current_root->data = s;
    current_root->left = NULL;
    current_root->right = NULL;
    return true;
  }
  else if (current_root->key == k)
    return false;
  else if (current_root->key > k)
    insert(current_root->left, k, s);
  else
    insert (current_root->right,k, s);
}

movieList.cpp

#include <iostream>
#include <stack>
#include <queue>
#include <string>
#include "tree.h"


using namespace std;

int main()
{
  Tree test;
  test.insert(100, "blah");
  return 0;
}

shouldn't `else if (root == k)` in insert be `else if (root->key == k)`. — Dan D., Apr 27 '11 at 02:18
@Dan yes it should, thanks for pointing that out, the problem is still there tho :/ — kingcong3, Apr 27 '11 at 02:24
You should post requests for code reviews at http://codereview.stackexchange.com . — Emile Cormier, Apr 27 '11 at 02:30

score 2 · Answer 1 · answered Apr 27 '11 at 02:34

2

Tree test(); is not how define an object of class Test, This acutally declare function named test which returns Tree.

try

Tree test; test.instert(100, "blah"); return 0;

answered Apr 27 '11 at 02:34

Suresh

183
1
1
5

Thanks, I called that at first, but I didnt know where the error came from so i started doubting myself and changed it – kingcong3 Apr 27 '11 at 03:07

score 1 · Answer 2 · answered Apr 27 '11 at 02:38

Couple of points:

You need to change the name of your member variable root to something else– I'd recommend m_root, or my_root, or tree_root, or something of those sorts. Right now you've got a little bit of a namespace clash in any function where you include root as an argument. This will also let you keep track of which root you're referring to.

bool Tree::insert(Node*& root, int k, string s)
{
    if(root == NULL){
        root = new Node;
        root->key = k;
        root->data = s;
        root->left = NULL;
        root->right = NULL;
        return true;
    } else
        if (root == k) //Comparison between pointer and number.
            return false;
        else
            if (root->key > k)
                insert(root->left, k, s);
            else
                insert (root->right,k, s);
    }

You need to change root on the commented line to root->key.

Other than that, it looks like it'll work.

EDIT: Also, what the other guy said. You declare an object as

TYPE name ()

if you are calling the default constructor (), so your code in your main function should be

Tree test;
test.insert(...)

Thanks, I changed it but theres still that error. Can u take a look again?? — kingcong3, Apr 27 '11 at 03:08
You probably need to add a "using namespace std;" line to your main file. It works fine on MacOS with GCC. EDIT: Also, make sure you compile your class file (g++ -c tree.cpp -> g++ -c movielist.cpp -> g++ -o a.out movielist.o tree.o) — Chaosphere2112, Apr 27 '11 at 03:30
i think the problem is in my helper call, but i dont really know how to call it correctly — kingcong3, Apr 27 '11 at 03:38
Ok, issue determined. You're not compiling the tree class. Instead of the "g++ movieList.cpp -o a.out" line, you need to do "g++ -c tree.cpp", and once that's done, "g++ -c movieList.cpp", then "g++ -o a.out movieList.o tree.o" — Chaosphere2112, Apr 27 '11 at 03:52

score 1 · Accepted Answer · answered Apr 27 '11 at 02:39

1

I copied some of your code and this is working fine for me:

main:

#include <iostream>
#include <stack>
#include <queue>
#include <string>
#include "tree.h"

    int main()
    {
      Tree test;
      test.insert(100, "blah");
      test.insert(50, "fifty");
      test.insert(110, "one hundred ten");
      return 0;
    }

Insert function:

bool Tree::insert(Node*& currentRoot, int k, string s)
{
  if(currentRoot == NULL){
    currentRoot = new Node;
    currentRoot->key = k;
    currentRoot->data = s;
    currentRoot->left = NULL;
    currentRoot->right = NULL;
    return true;
  }
  else if (currentRoot->key == k)
    return false;
  else if (currentRoot->key > k)
    insert(currentRoot->left, k, s);
  else
    insert (currentRoot->right,k, s);
}

Other than that you have syntax errors all over the place. I also changed the name because as someone pointed out there was a bit of a naming problem. CurrentRoot makes sense because you are passing it the root of the left or right subtree on every recursion.

answered Apr 27 '11 at 02:39

Pete

10,651
9
52
74

can u point our some of the syntax errors?? is it just the clashing of the name root?? – kingcong3 Apr 27 '11 at 02:50
Can u take a look again, i know my logic is sound, but theres something that F ing it up!! – kingcong3 Apr 27 '11 at 03:05
@kingcong3 I dont mind helping you out but you need to be a bit more specific. We don't need all of your code including unimplemented stuff. What are you trying to do that is not working? The code I provided is properly inserting my test data. – Pete Apr 27 '11 at 03:08
@Pete i cleaned up the codes a bit so it is easier to read. it is still giving the error. I am not sure what is wrong. So can you comment on whether or not I should use pointer as a reference or not like others suggested? – kingcong3 Apr 27 '11 at 03:19
@kingcong3 If I pass by reference, it works as it should as in my example code. I am sure someone will jump all over me if I am wrong but it has to be passed by reference otherwise you won't be able to change the pointer which we have to do. When currentRoot is NULL we assign it a pointer to a new Node. If you don't pass by reference then you are changing only a copy of that pointer. – Pete Apr 27 '11 at 03:24
@Pete, the code in the post is current, and i added back the reference but the error is still there. I am assuming that the problem is in the helper?? what is the correct way to use the helper? – kingcong3 Apr 27 '11 at 03:38
@kingcong3 I dunno. The error is in your movieList.cpp but I dont see any code for a movieList.cpp. – Pete Apr 27 '11 at 03:43
@Pete movieList is really just main, i just named it differently on here so just people know it is the main function that is doing everything – kingcong3 Apr 27 '11 at 03:52
@kingcong3 The answer here explains it better than I could. http://stackoverflow.com/questions/5745903/compile-c-in-linux – Pete Apr 27 '11 at 03:55

score 0 · Answer 4 · answered Apr 27 '11 at 04:01

0

Shouldn't you add tree.cpp to your build command?

[trinhc@cs1 Assignment_3]$ g++ movieList.cpp -o a.out

Would become

[trinhc@cs1 Assignment_3]$ g++ tree.cpp movieList.cpp -o a.out

answered Apr 27 '11 at 04:01

Drahakar

5,986
6
43
58

help with insert on first BST

4 Answers4