Problem either adding to or traversing a linked list

Question

I have a problem either adding to or traversing a linked list. The main item class is used by another class but I can add the correct number of these but it looks like when I add more data to the list the app no longer works.

I am not sure exactly where the error is. I know that when I try to traverse the list the application crashes. Any ideas or any improvements would be appreciated.

I can make the crash not happen by changing the AddOccurence method to not do the while loop.

Do

void Item::AddOccurence(int Item,int placeInLine){
    ItemOccurence* ocr=myHead;
    if(ocr)
    {

    }

instead of

void Item::AddOccurence(int Item,int placeInLine){
    ItemOccurence* ocr=myHead;
    while(ocr)
    {

    }

Basically hitting the first node but no more.

I have an object that contains a list. Here is the .h file #include using namespace std;

class ItemOccurence{
public:
    ItemOccurence(int line,int placeInLine,ItemOccurence* link=NULL) :myLine(line),myPlaceInLine(placeInLine),myLink(link){}

    int myLine;
    int myPlaceInLine;
    ItemOccurence* myLink;
};

class Item {
public:
    Item();
    Item(string Item,int line,int placeInLine);
    virtual ~Item();
    void deallocate(ItemOccurence* p);
    void AddOccurence(int Item,int placeInLine);
    string myItem;
    ItemOccurence* myHead;
private:
    bool isEmpty();
};

And the .cpp file

#include "Item.h"
#include <string>
#include<iostream>
using namespace std;
Item::Item(string Item,int line,int placeInLine):myHead(NULL){
    myItem=Item; 

    myHead= new ItemOccurence(line,placeInLine,NULL);
}

Item::Item():myHead(NULL){
myHead=0;
}

Item::~Item() {
    deallocate(myHead);
    myHead=0;
}

void Item::deallocate(ItemOccurence* p){
    ItemOccurence* tmp;
    while(p){
        tmp=p;
        p=p->myLink;
        delete tmp;
    }
}

void Item::AddOccurence(int Item,int placeInLine){
    ItemOccurence* ocr=myHead;
    while(ocr)
    {
          cout<<"orrucence head while adding " << myHead->myLine << " " << myHead->myPlaceInLine <<"\n";
        ocr=ocr->myLink;
    }

    myHead = new ItemOccurence(Item,placeInLine,myHead);

    return;
}

bool Item::isEmpty(){
    if(myHead)
        return false;
    else
        return true;
}

EDIT: I updated AddOccurence to be.

void Item::AddOccurence(int line,int placeInLine){
    ItemOccurence* prev = myHead;
    ItemOccurence* curr = myHead->myLink;

    while(curr){
        prev=curr;
        curr=curr->myLink;
        }

    // insert new ItemOccurence
    cout<<"adding " <<line<< " and " << placeInLine <<"\n";
    prev->myLink = new ItemOccurence(line,placeInLine); 

    return;
}

But I am still crashing. I am trying to debug but not sure what to look for.

Answer 1

It's very hard to tell what your code is trying to do. Unfortunately, the hard truth is, it's pretty far away from "working".

Here's a few hints:

• Reconsider your classes. What is Item and ItemOccurrence? A linked list is a list. It has items. You should probably name it List and Item. If you want, you can do this in a single class representing Item (the List is only a special case of the front Item).
• Each Item needs a string (the node data) and a next (the pointer to the next node).
• If you use List, it will need to have a pointer to head (the first Item).
• You don't need to store placeInLine. placeInLine should only be used when searching for the place to insert a new Item.
• It's unclear what ItemOccurrence::myLine is supposed to represent.
• When you initialize myHead(NULL), you don't need to set it to 0.
• isEmpty() is not usually a private method.
• The algorithm for adding a node is:
  • Loop until you find the place before it needs to be inserted.
    • It looks like you need to check for two things: "end of list" and "placeInLine"
  • Set new_node->next = current->next->next node.
  • Set current->next = new_node node.
  • Be aware of the special case where current->next is NULL.
• Instead of if (x) return true; else return false;, it's common to return x;

Answer 2

There are several issues with the code, but most notably your addOccurrence method does not set the myLink pointer for myHead after it creates it. I think that's what you are trying to do in the loop above it, but in actuality that code only seems to loop through the existing list and print it out. You probably intend to loop through that placeInLine times and then update your myLink pointers at that level. It's not clear what the difference is between item and placeInLine or why you would need both. You definitely don't want to use class names (like Item) as integer variables. That adds to the confusion.