0

I'm building an implementation of a skip list in C++ with quad links to surrounding nodes.

Here is the skip list node class:

class SkipList_Node {
public:
    int key;
    string data;

//special values for infinity keys
int pos_infinity_key = 1000;
int neg_infinity_key = -1000;

//pointers to surrounding nodes
SkipList_Node *up;
SkipList_Node *down;
SkipList_Node *left;
SkipList_Node *right;

//Constructors
SkipList_Node() {};

SkipList_Node(int k, string d) {
    this->key = k;
    this->data = d;

    up =  NULL;
    down = NULL;
    left = NULL;
    right = NULL;

}
};

Here is the Skip list class.

class Skip_List {
public:
    //first and last nodes in the list
    SkipList_Node first;
    SkipList_Node last;

//Number of rows in the list
int height;

//Constructor for an empty skiplist
Skip_List() {
    //initialize infinity keys
    SkipList_Node node1(SkipList_Node().neg_infinity_key, ""); 
    SkipList_Node node2(SkipList_Node().pos_infinity_key, "");

    //link new nodes
    node1.right = &node2;
    node2.left = &node1;
    //update first and last key values
    first = node1;
    last = node2;

    height = 0;
}

//Randomly determine the height of a node   
int getLevel() {
    //set the seed for random function
    srand(time(NULL));

    //Simulate coin flips and store heads into array
    //Max height is 4
    int flips[5];
    for (int i = 0; i < 5; i++) {
        flips[i] = 1 + rand() % 2;
    }

    //Determine height based on heads
    //As soon as tails is fliped stop counting height
    int height_counter = 0;
    for (int n = 0; n < 5; n++) {
        if (flips[n] == 2) {
            height_counter += 1;
        }
        else {
            break;
        }
    }

    return height_counter;

}

//return height of the skip list
int getHeight() {
    return height;
}

//find the largest key which is <= value of key passed
SkipList_Node searchSkipList(int k) {
    //pointer to the node
    SkipList_Node* ptr;
    //point to the first node of the list
    ptr = &first;
    //move right until a key <= value of key passed is found
    while (1) {
        while (ptr->right->key != SkipList_Node().neg_infinity_key &&
            ((ptr->right->key) - k) <= 0) {

            ptr = ptr->right;
        }

        //drop down level
        if (ptr->down != NULL) {
            ptr = ptr->down;
        }
        else {
            //Lowest level of skip list reached
            break;
        }
    }

    return *ptr;
}

//Return the value of a node given the key
string getValue(int k){
    SkipList_Node* ptr;
    //search for the node
    ptr = &searchSkipList(k);

    //If key is found return its value else return null
    if (k == (ptr->key)) {
        return ptr->data;
    }
    else {
        cout << "No Nodes found with that key value";
        return NULL;
    }
}

//Insert a new node into skip list
void insertIntoSkiplist(int k, string d) {
    SkipList_Node* p;
    int level;

    p = &searchSkipList(k);

    //if key already exists update the value
    if (k == (p->key)) {
        p->data = d;
        return;
    }

    // Create and insert a new node
    SkipList_Node* q = new SkipList_Node(k, d);
    q->left = p;
    q->right = p->right;
    p->right->left = q;
    p->right = q;

    //Calculate the level
    level = getLevel();

    //insert node in each level
    for (int i = 0; i <= level; i++) {
        //Create new empty row if level is hiegher than the current height of the list
        if (level >= height) {
            SkipList_Node* node1 = new SkipList_Node(SkipList_Node().neg_infinity_key, "");
            SkipList_Node* node2 = new SkipList_Node(SkipList_Node().pos_infinity_key, "");

            //link inifnity nodes
            node1->right = node2;
            node1->down = &first;
            node2->left = node1;
            node2->down = &last;
            first.up = node1;
            last.up = node2;
            first = *node1;
            last = *node2;
        }

        while (p->up == NULL) {
                p = p->left;
        }

        p = p->up;

        SkipList_Node* z = new SkipList_Node(k, NULL);

        z->left = p;
        z->right = p->right;
        z->down = q;

        p -> right -> left = z;
        p->right = z;
        q->up = z;

        q = z;

        level += 1;

    }
}
};

In the main I create a new skip list object. Looking at the debugger, the constructor successfully creates node1 and node2 and uses them to update nodes first and last. At this point nodes first and last have the correct linked values, First.right has key 1000 and last.left has key -1000.

I then call the insertIntoSkipList() method which in turn calls the searchSkipList() method. Looking at the debugger at the point where I assign the memory address of first to pointer ptr, the values of first.right are now key= -858993 and data = "Error reading characters of string". I'm not sure why the memory address of first holds these values for the pointer to the right node. At this point I expect first.right to still hold values key = 1000 and data="" which will then be pointed to by ptr->right.

I do not have much experience with C++ and pointers.

JustAProgram
  • 37
  • 1
  • 6
  • This is probably a good time to [learn how to debug your programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). – Some programmer dude Mar 19 '19 at 19:39
  • A possible hint though: After you do `ptr = &searchSkipList(k);` the object that `searchSkipList` returned will be destructed. The pointer you have is no longer valid. Do you *have* to use pointers there? – Some programmer dude Mar 19 '19 at 19:40
  • Unrelated: You almost always want to call `srand` only once somewhere near the beginning of the program. If you do have one of the ratre cases where you do need to call `srand` multiple times, odds are very good that `rand` is not the random number generator you need to use. – user4581301 Mar 19 '19 at 19:45

1 Answers1

0

You are assigning your left and right pointers to the address of a temporary object. They go out of scope as soon as the SkipList constructor runs and you now have dangling pointers.

You need to allocate memory if you need a permanent reference. Take a look at pointers and memory over at cplusplus.com :

http://www.cplusplus.com/doc/tutorial/pointers/ http://www.cplusplus.com/doc/tutorial/dynamic/

BugSquasher
  • 307
  • 1
  • 12
  • For a different view, [Why should C++ programmers minimize use of 'new'?](https://stackoverflow.com/questions/6500313/why-should-c-programmers-minimize-use-of-new) – user4581301 Mar 19 '19 at 19:50