0

I'm working on a C++ assignment where I'll create a search engine on a linked list of linked lists. As per the requirements, I can't use other libraries nor STL.

Basically it will be like this (I removed the variables from small list since they are irrelevant): enter image description here

My structs are these:

struct small
{
    int data;
    struct small *next;
};


struct big
{
    int playerID;
    string playerName;
    string playerTeam;
    struct small *goals;
    struct big *next;
};

Here's the relevant code snippet, I think the problem is at addGoals(...) where I'm failing to assign the small element to the temp->goals.

class biglist
{
private:
    big *head, *tail;

public:
    biglist()
    {
        head = NULL;
        tail = NULL;
    }

. . .

void createbig(int ID, string name, string team)
    {
        big *temp = new big;
        temp->playerID = ID;
        temp->playerName = name;
        temp->playerTeam = team;
        temp->goals = NULL;
        temp->next = NULL;

        if (head == NULL)
        {
            head = temp;
            tail = temp;
            temp = NULL;
        }
        else
        {
            tail->next = temp;
            tail = temp;
        }
    }

void addGoals(int id, small *s)
    {
        big *temp = head;
        while (temp != NULL)
        {
            if (temp->playerID == id)
            {
                temp->goals = s;
                break;
            }
            temp = temp->next;
        }
    }

    void test()
{
    big *temp = head;
    while (temp != NULL)
    {
        if (temp->playerID == 1)
        {
            if (temp->goals !=NULL)
            {
                cout << temp->goals->data << endl;
            }
            else
            {
                cout << "goals null" << endl;
            }
        }
        temp = temp->next;
    }
}
}
. . .

 

class smalllist
{
private:
    small *head, *tail;

public:
    smalllist()
    {
        head = NULL;
        tail = NULL;
    }

    void createsmall(int ID, biglist b)
    {
        small *temp = new small;
        temp->data = ID;
        temp->next = NULL;

        if (head == NULL)
        {
            head = temp;
            tail = temp;
            temp = NULL;
        }
        else
        {
            tail->next = temp;
            tail = temp;
        }


        b.addGoals(1, temp);
    }
};

Finally, my main code:

int main()
{
    biglist obj;
    obj.createbig(1, "Player1", "Team1");
    obj.createbig(2, "Player2", "Team2");
    obj.displaybig();

    smalllist sml;
    sml.createsmall(9, obj);
    sml.displaysmall();

    obj.displaybig();
    obj.test();
}

Debugging throws an exception at:

cout << temp->goals->data << endl;

saying that

Exception thrown: read access violation. temp->goals was nullptr.

I'm 90% sure I messed up something with pointers; but other stuff I've tried gave errors before compiling. I checked out some books / tutorials but couldn't figure it out.

Also if you have a better approach or saw one of the horrible mistakes that I'm making, please don't hold back :)

Thanks.


EDIT I changed my createbig() like this. Currently it works with following codes:

void createbig(int ID, string name, string team, small *s)
    {
        big *temp = new big;
        temp->playerID = ID;
        temp->playerName = name;
        temp->playerTeam = team;
        temp->goals = s;
        temp->next = NULL;

        if (head == NULL)
        {
            head = temp;
            tail = temp;
            temp = NULL;
        }
        else
        {
            tail->next = temp;
            tail = temp;
        }
    }

and added this to small

small getsmall(int i)
    {
        small *temp = head;
        while (temp != NULL)
        {
            if (temp->data == i)
            {
                return *temp;
            }
        }
    }

My final main function is

int main()
{
    smalllist sml;
    sml.createsmall(9);
    sml.displaysmall();

    biglist obj;
    small s = sml.getsmall(9);
    obj.createbig(1, "Player1", "Team1", &s);
    //obj.createbig(2, "Player2", "Team2");
    obj.displaybig();

    obj.test();

}

While it ends successfully now, it gives the address of goals and I get this in debug section:

enter image description here

Aguen
  • 17
  • 1
  • 6
  • You have to check if this is a nullptr before dereferencing. – drescherjm Nov 30 '18 at 19:37
  • Does something call `addGoals()`? – drescherjm Nov 30 '18 at 19:38
  • In `addGoals()` you can break after `temp->goals = s;` no need to keep iterating if the id was already found. – drescherjm Nov 30 '18 at 19:40
  • @drescherjm I added the part where I call addGoals() and added your suggestion to addGoals(). – Aguen Nov 30 '18 at 19:43
  • Doesn't `b` go out of scope in `createsmall()`? – drescherjm Nov 30 '18 at 19:46
  • 1
    Where is the source for `createbig`? (Isn't that the call that is supposed to create the item in the big list that `test` is trying to output?) – JaMiT Nov 30 '18 at 19:47
  • Also, memory leaks each time this code says `new big` (outside `createbig` which was just added as I was typing). – JaMiT Nov 30 '18 at 19:50
  • I replaced them with `big *temp;`, was that what I should do? Still getting the same error by the way. – Aguen Nov 30 '18 at 19:56
  • `big *temp = new big; temp = head;` is a bug in many functions. No need to allocate a new node if you are just using a pointer to some other node. just use `big *temp = head;` instead for this. – drescherjm Nov 30 '18 at 19:56
  • The following code in `createsmall()` does nothing useful: `biglist b; b.addGoals(1, temp);` the reason why I say that is after the `}` the variable `b` no longer exists. Maybe you thought this would modify some other `biglist` object that you had previous created. I can tell you that it does not. `b` is a separate object that only exists in the scope of this function. – drescherjm Nov 30 '18 at 20:06
  • Yeah, added a nullptr check and you're right. How do I access to those "temp" elements out of createsmall() though – Aguen Nov 30 '18 at 20:08
  • You need to somehow pass the `biglist` that you are using to createsmall() preferably by reference. Without it createsmall() can not change `obj` – drescherjm Nov 30 '18 at 20:12
  • I think I did it, but still having the same error. I think I'll try to initialize each big list with a small list instead of declaring goals as null. – Aguen Nov 30 '18 at 20:29
  • 0xcdcdcdcd is an uninitialized heap pointer. https://stackoverflow.com/questions/127386/in-visual-studio-c-what-are-the-memory-allocation-representations – drescherjm Nov 30 '18 at 21:29
  • `temp->goals = &s;` is wrong. You can't take the address of a temporary and store that. when `s` goes out of scope `temp->goals` points to garbage. – drescherjm Nov 30 '18 at 21:31
  • That usage is the same as this: https://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope/6445794#6445794 – drescherjm Nov 30 '18 at 21:36

2 Answers2

1

You don't seem to have added any goals, so I'm assuming the code initializes with null. and so the nullptr exception.

call addgoals() with the goals to player before test().

the other suggestions would be

  1. to add a null check before printing goals
  2. temp pointers need not be initialized with new big or small just the head of the list would be enough
Deepan
  • 124
  • 1
  • 10
  • I updated with your suggestions. `addGoals()` is called inside `createsmall()` though, so it was called before `test()` But apparently it doesn't do anything due to scope. – Aguen Nov 30 '18 at 20:11
1

Let's look at what your code does, going through the main function. (Being able to walk through code like this is a useful skill. You can also use a debugger to help out, stepping through your function line-by-line.)

biglist obj;

Default construct a biglist. The head and tail are null. (By the way, nullptr is C++'s replacement for C's NULL.)

obj.createbig(1, "Player1", "Team1");
obj.createbig(2, "Player2", "Team2");

Add entries in obj for players with IDs 1 and 2. Their goals are null.

obj.displaybig();

Presumably an output of obj?

smalllist sml;
sml.createsmall(9);
sml.displaysmall();

These lines do something with a smalllist, but do not reference obj, so they are not relevant to this issue.

obj.displaybig();

Presumably an output of obj? Kind of redundant since nothing affected obj since the last display.

obj.test();

Call the test code, which finds the element for player ID 1 and outputs the data of that player's first goal. However, if you look up where that player was added, the goal is null, so you get a crash.


Separate from the above, there is probably some confusion in createsmall. Inside that function, a new biglist is created (not obj), and that list is told to add a goal to the player with ID 1. However, this has no effect the biglist in the main function.

JaMiT
  • 14,422
  • 4
  • 15
  • 31
  • Thanks forn the detailed reply. Second `displaybig()` was to check if the results were same. Since I thought `addGoals()` called from `createsmall()` would change `obj` – Aguen Nov 30 '18 at 20:15