0

I start with the top node of a tree 5 layers deep, and recursively call getvalue() on each one. Each node is linked to two nodes on the next layer. I'm pretty sure that this is not my issue since I double checked the algorithm on paper. Once I get to layer 3, however, it gives me a segmentation fault. With valgrind, I figured out that it was raised when I tried to print the class variable oper. I have no idea where to go with this, so your help is greatly appreciated. This is the code:

class Node {
    public:
        vector<Node> children;
        long constval;
        char oper;
        void setconst();
        Node();
        void copy(const Node*);
        int getvalue();
    private:
        int mult(int,int);
        int div(int,int);
        int add(int,int);
        int sub(int,int);
};

Node::Node() {
    bool c = false;
    vector<char> operations;
    operations.push_back('m');
    operations.push_back('a');
    operations.push_back('s');
    operations.push_back('d');
    operations.push_back('c');
    constval = rand();
    int randnum = rand() % 5;
    cout << randnum << "\n";
    oper = operations[randnum];
}

int Node::getvalue() {
    cout << oper << '\n';
    if (oper == 'm') {
        return Node::mult(children[0].getvalue(), children[1].getvalue());
    }
    else if (oper == 'd') {
        return Node::div(children[0].getvalue(), children[1].getvalue());
    }
    else if (oper == 'a') {
        return Node::add(children[0].getvalue(), children[1].getvalue());
    }
    else if (oper == 's') {
        return Node::sub(children[0].getvalue(), children[1].getvalue());
    }
    else if (oper == 'c') {
        return constval;
    }
}

EDIT: Here is my initializing algo:

class Individual {
    public:
        vector< vector<Node> > nodes;
        vector< vector<Node> > getrand();
        void replace(vector< vector<Node> >);
        void mutate(double);
        double run();
        Individual();
};

Individual::Individual() {
    nodes.resize(5);
    nodes[0].resize(1);
    int size = 2;
    for(int i = 1; i < 5; i++) {
        nodes[i].resize(size);
        size = size * 2;
    }
    vector<char> operations;
    operations.push_back('a');
    operations.push_back('s');
    operations.push_back('d');
    operations.push_back('m');
    nodes[0][0].oper = operations[rand() % 4];
    for(int x = 0; x < nodes[4].size(); x++) {
        nodes[4][x].setconst();
    }
    for(int i = 0; i < 4; i++) {
        for(int x = 0; x < nodes[i].size(); x++) {
            nodes[i][x].children.push_back(nodes[i+1][x*2]);
            nodes[i][x].children.push_back(nodes[i+1][x*2+1]);
        }
    }   
}
1
  • Where are you pushing elements to the class member vector<Node> children; ? In the constructor, you are pushing elements to a local variable. Printing of oper has nothing to do with segmentation fault.
    – Mahesh
    Sep 13, 2011 at 4:37

3 Answers 3

1
vector<Node> children;

I have the same opinion as Andrey; that I also don't like the use of vector as child nodes container. If your data structure is a simple binary tree, why not simply use

Node* leftChild;
Node* rightChild;

as the data members of class Node?

Also, please provide the code for the creation of your tree. It is likely that you have made some mistake there since segmentation fault is very likely due to improper creation of data structure and you may not have realised it.

5
  • better yet is std::pair<std::unique_ptr<Node>, std::unique_ptr<Node>> Sep 13, 2011 at 7:38
  • @Alexandre: I don't see the benefit of placing them in a pair, but we agree on the unique_ptr.
    – GManNickG
    Sep 13, 2011 at 7:48
  • Thank you so much ksming! That was a very easy fix. Sep 14, 2011 at 4:37
  • @ksming: Though I don't think this is a very good advice for beginners (and not even intermediates). Having bare pointers around puts you in the position of making your code exception safe (including construction and destruction) and writing correct copying constructor and assignment (or forbidding them entirely, which is often more of an antifeature) without leaking memory or invoking undefined behavior. Sep 14, 2011 at 5:15
  • @phresnel: I absolutely agree with you with regards to usage of bare pointers. My answer is only a suggestion and because I have little experience with smart pointers, I couldn't provide a more correct answer. In any case, I think it is purely coincidental that my suggestion solved OP's problem so easily. I'm sure the REAL problem lies somewhere else, which is up to OP to find out.
    – ksming
    Sep 14, 2011 at 5:44
0
  1. On first look, it seems that your tree's last layer must contain the numbers only. But this is not guaranteed by the algo.

  2. Actually, I don't like vectors as children container. Why not to use two pointers? Also, I don't see where you initialize the vector and put children into it.

  3. Take a look at how rand() works.

1
  • The first is guaranteed by the algorithm because all nodes have a constval member, and it gets assigned in the node's constructor with a random number. Sep 13, 2011 at 4:29
0

You are never checking whether your node has any children.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.