1

I don´t get this, when I call head->valueit returns me the last value added to the linked list. Should it not return me the first item since head is only set when it is empty? correct? I am thinking maybe there is there some other bug in the code.

void LinkedListPQueue::enqueue(const string& elem) {
    cell *newCell = new cell;
    newCell->value = elem;
    newCell->next = NULL;
    if(this->isEmpty()) {
        this->head = this->tail = newCell;
    } else {
        // find smallest and put it there

        this->tail->next = newCell;
        this->tail = newCell;
    }
}

declared in header

struct cell {
    std::string value;
    cell *next;
};
cell *head, *tail;
Tom Lilletveit
  • 1,872
  • 3
  • 31
  • 57
  • 1
    There's no `this->value` in the code you've shown. Post a small but complete program that shows the problem. – Pete Becker Jan 12 '13 at 14:19
  • Have you tried stepping through the code line by line in a debugger? Are you sure the problem lies in this `enqueue` function, and not the function where you print the values or somewhere else? – Some programmer dude Jan 12 '13 at 14:21
  • 1
    // find smallest and put it there -> That is the important part, without that nobody can help directly. But I'm pretty sure, there is the problem. Also take a look at your isEmpty(). – Peter Jan 12 '13 at 14:22
  • @Peter I guess there is no more code there. `this->tail->next = newCell;this->tail = newCell;` is enough. – Agent_L Jan 12 '13 at 14:27
  • If you are going to implement your own datastructures then I strongly recommend you to have a look at the [RAII](http://stackoverflow.com/questions/395123/raii-and-smart-pointers-in-c) and [copy-swap](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) posts. – StackedCrooked Jan 12 '13 at 14:29
  • @Peter: They are notes to self. – Tom Lilletveit Jan 12 '13 at 14:29
  • 2
    Okay. But what means "find smallest" - that doesn't make sense in the context. And whatever your "counting", I'm pretty sure your shouldn't do that in a linked-list. – Peter Jan 12 '13 at 14:48

1 Answers1

6

My be isEmpty is not implemented correctly so everytime you add a new node you re-assign the head to that node

hmatar
  • 2,437
  • 2
  • 17
  • 27
  • Actually you are correct, I forgot updating the counter. I am a beginner on the use of pointers and linked list, so that´s why I thought initially it was a problem there – Tom Lilletveit Jan 12 '13 at 14:28
  • @pmr unless you can think of another alternative, guessing from set of 1 is 100% sure – Agent_L Jan 12 '13 at 14:30
  • 1
    @TomLilletveit: `bool isEmpty(){return null==head;}`. No counters. – Agent_L Jan 12 '13 at 14:31
  • @Agent_L It's beginner C++ code. I can think of a thousand alternatives. – pmr Jan 12 '13 at 14:31
  • Agent_L: That´s what I thought as well, it´s a linked list - it needs lots of pointers. – Tom Lilletveit Jan 12 '13 at 14:34
  • @pmr: this does not make the answer less insightful – Andy Prowl Jan 12 '13 at 14:34
  • 1
    @Agent_L I'm sorry you didn't see the humor. You are right, all pointers here are needed. Still, this should be a comment and not an answer (guesses, even educated ones, belong there). OP should provide a minimal example exhibiting the problem before this is actually answerable. – pmr Jan 12 '13 at 15:04
  • @pmr You're right about that - it should be a comment. Hassan gambled and won. – Agent_L Jan 12 '13 at 15:24