Linked Lists error: m_deck undeclared identifier

My compiler's throwing a fit because m_deck is an undeclared identifier. But I think I DID declare it: Please tell me what you think from the following snippets:

I have declared card data structures in a card.h header file.
I have declared a list of cards as a nonmember function in my deck.h header file.

Code:

---

typedef std::list<s_card> cardList_t;

---

and I have declared a cardList_t named m_deck in my game.h header file.

Code:

---

cardList_t m_deck;

---

Then I use it in my game.cpp definition file.

Code:

---

void shuffleDeck()
{
        cardNodeptr cur = head;

        int x, y;

        for(int count=1; count<=52; count++)
        {
                x = 1+rand()%52;
                y = 1+rand()%52;

                m_deck.insert(y,m_deck.retrieve(x));
                m_deck.removeCard(x);
        }
}

---

Thanks as always for your time!

You forgot to put the classname before the function name.

Okay, so I've made some changes. Now VS is giving me errors about 'head undeclared identifier' - which I can see because I defined it in the card class header file and used it in the deck class definition file. How do I fix that? Here is a copy of what I'm doing:

Code:

---

//card.h

#pragma once

struct s_card
{
        int value;
        char suit;
};

struct cardNode
{
        s_card item;
        cardNode * next;
};

typedef cardNode * cardNodeptr;


class CCard
{

public:
        CCard();
//        void display();
//        int aceValue();


protected:
        cardNodeptr head;

};

---

Code:

---

//deck.h
#pragma once
#include "card.h"
#include <list>



class CDeck
{

public:
        CDeck();
        bool empty();
        void insert(int index, s_card c);
        void insertAtHead(s_card c);
        void displayCard();
        int aceValue();
        s_card retrieve(int index);
        void removeCard(int index);


private:

       
};

typedef std::list<s_card> cardList_t;

---

Code:

---

//deck.cpp

#include "card.h"
#include "deck.h"

#include <iostream>

CDeck::CDeck()
{}

bool CDeck::empty()
{

        return(head==NULL);
}

void CDeck::insert(int index, s_card c)
{
        if(empty()||index==1)
        {
                insertAtHead(c);
        }
        else
        {

                cardNodeptr cur = head;
                int nodenum = 1;
                while(cur != NULL && nodenum < index)
                cur = cur->next;
                nodenum++;
        }
}

void CDeck::insertAtHead(s_card c)
{

        cardNodeptr popEye = new cardNode;
        popEye->next = head;
        popEye->item = c;
        head = popEye;
}

s_card CDeck::retrieve(int index)
{
        s_card card;

        cardNodeptr cur = head;

        for(int x=1; x<=index; x++)
        {
                cur = cur->next;
        }
        return card;
}

void CDeck::removeCard(int index)
{

        cardNodeptr cur = head;
        cardNodeptr prev = NULL;

        if(index==1)
        {
                head = head->next;
        }
        else
        {
                for(int x=1; x<=index; x++)
                {
                        prev = cur;
                        cur = cur->next;
                }
        }
        delete cur;       
}

---

What should I do? When I move the cardNodeptr head to outside the class as a nonmember function, I get errors about 'struct cardNode head already defined'

If you are using std::list, you don't need CardNode and you don't need CardNodePtr and you don't need head.

Just make a cardList_t member in your CDeck class. (I might use a vector instead of a list, though for easier shuffling).


If you're trying to make your own linked list for learning purposes, then you don't need std::list. Only do this for learning purposes, though. If you're not specifically trying to learn how to build your own linked list, then stick with std::list and get rid of all that extra stuff.

Thanks for that! Okay, so I cleaned it all up. Actually I scrapped it and started over from stubs. Now I am attempting to shuffle a deck of cards, and have an issue. Code first, explanation of issue after:
Header:

Code:

---

#pragma once
#include "card.h"

class CDeck : public CCard
{
public:

        CDeck();
        bool empty();
        void insert(int index, s_card c);
        void insertAtHead(s_card c);
        void retrieve(int index, s_card c); // <- This is the function in question
        void remove(int index);

        void buildDeck();
        void shuffleDeck();

protected:
        int size();


};

---

Partial Definition:

Code:

---


void CDeck::insert(int index, s_card c)
{
        if(empty()||index==1)
        {
                insertAtHead(c);
        }
        else
        {
                cardNodeptr cur = head;
                int nodenum = 1;
                while(cur != NULL && nodenum < index)
                {cur = cur->next;}
                nodenum++;
        }
}

void CDeck::retrieve(int index, s_card c)
{

        cardNodeptr cur = head;
        for(int x=1; x<=index; x++)
        {cur = cur->next;}
        c = cur->item;
}

void CDeck::remove(int index)
{
        cardNodeptr cur = head;
        cardNodeptr prev = NULL;

        if(index==1)
        {head = head->next;}
        else
        {
                for(int x=1; x<=index; x++)
                {
                        prev = cur;
                        cur = cur->next;
                }
        }
        delete cur;
}

// and the problematic bloc:
void CDeck::shuffleDeck()
{
        int x,y;


        for(int count=1; count<=52; count++)
        {

                x = 1+rand()%52;
                y = 1+rand()%52;

                insert(y,retrieve(x,));//<- What goes in retrieve's second parameter?
                remove(x);
        }
}

---

I suspect I am not using pointers correctly. If I were to call retrieve() from main(), I could just stick an object in there. But I'm not doing that. So what goes in there? How do I tie these pieces together?

So it looks like you're trying to learn how to make a linked list.

I would not derive CDeck from CCard. That's not what public inheritance is for. Instead I'd keep the CardNodePtr for the head of the list inside the CDeck class.

Your retrieve function has a problem. It's supposed to return data, but it doesn't. Normally you would return an s_card from the function. If you want to leave it the way you have it, where you assign the new value to c, then that should be a reference parameter. Otherwise, the calling code won't get the value (because c would be a copy of the calling code's variable).

As for your shuffle code, that won't really be a very good shuffle. There are better algorithms for shuffling. Basically, you want to get a random number between 1 and 52, then swap the first card with the card at that location. Then get a random number between 2 and 52 and swap the 2nd card with the card at that location. And so on until you get to 52. This ensures a complete and random shuffle.

I have had so much trouble with LL that I need to learn this once and for all from scratch.
I removed the public class inheritance, and moved the cardNodeptr to CDeck. I changed the return type on retrieve to s_card and eliminated the second parameter on retrieve. And I fixed the shuffle algorithm based on your suggestion.

Code:

---

s_card CDeck::retrieve(int index)
{


        cardNodeptr cur = head;
        for(int x=1; x<=index; x++)
        {cur = cur->next;}
        return cur->item;
}

---

changed some other things around... and fixed that error message and pulled my hair out while an unhandled exception occurred in .exe. hm.

Unhandled exception has a problem with my buildDeck function below

Code:

---

void CDeck::buildDeck()
{
        cardNodeptr cur = head;

        for(int x=1; x<=4; x++)
        {
                for(int y=1; y<=13; y++)
                {
                        if(x==1)
                        {cur->item.suit = 'H';}
                        else if(x==2)
                        {cur->item.suit = 'S';}
                        else if(x==3)
                        {cur->item.suit = 'D';}
                        else if(x==4)
                        {cur->item.suit = 'C';}

                        cur->item.value = y;

                        cur->next;
                }
        }
}

---

because value, suit, and next cannot be evaluated.
Now, the card structure is defined in a different class. Do I need to combine classes or is there a better method of access?

Where do you create new cards? When you start with your deck, the head of the list is uninitialized. Hopefully you initialize it to null in your constructor. You then have to build the list by creating new nodes and assigning to the next pointer of the previous node. You can do that in the constructor, or you can do that in the buildDeck function. (I would actually do it in the buildDeck function and then call buildDeck from the constructor.)

So do you have any code that calls new to allocate a new card? If not, that's what you've got to do. Otherwise you're accessing cards that don't exist.

I have done this in the constructor

Code:

---

CDeck::CDeck:head(0)

---

I have a function written for insert at head, which assigns new nodes, which I forgot to use in my buildDeck. That will probably work better. I'll come back and say what happened.

Code:

---

void CDeck::insert(int index, s_card c)
{
        if(empty()||index==1)
        {
                insertAtHead(c);
        }
        else
        {
                cardNodeptr cur = head;
                int nodenum = 1;
                while(cur != NULL && nodenum < index)
                {cur = cur->next;}
                nodenum++;
        }
}

---

Well, this function doesn't do anything useful in the else part...

It's purpose is also a little vague: I suppose it is meant to change the contents of node at index, except if index is 1 and the list is empty it does the same thing as insertAtHead? This is probably not the kind of behaviour one would expect from a function called insert (which might be expected to insert new nodes into the list).

------------
If you want index-based (random) access to contents, a linked list is not a suitable container. That's what arrays and std::vector are for.

---------------
It is also not that good an idea to implement something low level like a linked list in a class that has high-level tasks (such as CDeck). Consider this: normally a program has several classes that contain some kind of list (or lists) of data. Would you think it is wise to implement a linked list from scratch for all of them?

Instead CDeck might use a suitable container to hold its card data (as a private member), and concentrate only on its real tasks, such as shuffling etc.

I know, I know...but I really need to learn all about LL's.
I did start out defining a separate card class, but was hampered by my lack of knowledge on how to relate the classes w/o using public inheritance, so I finally rolled it all into one class.

How about this for the insert function?

Code:

---


void CDeck::insert(int index, s_card c)
{
        if(empty()||index==1)
        {
                insertAtHead(c);
        }
        else
        {
                cardNodeptr cur;
                cardNodeptr prev;
                cur = head;
                prev = NULL;

                for(int x=1; x<=index; x++)
                {
                        prev = cur;
                        cur = cur->next;
                }
                cardNodeptr insert;
                insert->next = cur;
                prev->next = insert;
                delete cur;
        }
}

---

And what's going on with my takeTurn function? VS says that cvalue is being used w/o being initialized.

Code:

---

int CDeck::takeTurn()
{
        cardNodeptr cur;

        int cvalue;

        char choice = 'H';

        while(toupper(choice=='H'))
        {

                std::cout << "Please select (H)it or (P)ass: ";
                std::cin >> choice;

                if(toupper(choice=='H'))
                {
                        display(cur->item);
                        if(cur->item.value==1)
                        {cvalue=aceValue();}
                        else
                        {cvalue = cur->item.value;}
                }
        }
        return cvalue;
}

---

I made some changes to my takeTurn(), and get the same runtime error message.

Code:

---

int CDeck::takeTurn()
{
        s_card c;
        int cvalue;
        char choice = 'H';

        while(toupper(choice=='H'))
        {

                std::cout << "Please select (H)it or (P)ass: ";
                std::cin >> choice;

                if(toupper(choice=='H'))
                {
                        c = retrieve(1);
                        display(c);
                       
                        if(c.value==1)
                        {cvalue = aceValue();}
                        else
                        {cvalue = c.value;}
                }
        }
        remove(1);
        return cvalue;
}

---

But at least this bloc looks a tad bit cleaner.

You should initialize cvalue. What if the user passes? What will the function return? What should it return?

>> I made some changes to my takeTurn(), and get the same runtime error message.
Are you talking about the variable being used without being initialized, or some other error message?

Oh.
I've been staring at this for so long I've stopped recognizing error messages. I initialized cvalue to 0.
I'm sure I'll be back on in a few minutes.

I have a new unhandled exception, and I think it's coming from my insert function which is defined as follows:

Code:

---

void CDeck::insert(int index, s_card c)
{
        if(empty()||index==1)
        {
                insertAtHead(c);
        }
        else
        {
                cardNodeptr cur = head;
                cardNodeptr prev = NULL;

                for(int x=1; x<=index; x++)
                {
                        prev = cur;
                        cur = cur->next;
                }
                cardNodeptr newPtr = cur;
                newPtr->item = c;
        }
}

void CDeck::insertAtHead(s_card c)
{
        cardNodeptr cur = new cardNode;
        cur->next = head;
        cur->item = c;
        head = cur;
}

---

It might be helpful to know how I typedef my pointers:

Code:

---


struct s_card
{
        int value;
        char suit;
};

struct cardNode
{
        s_card item;
        cardNode *next;
};

typedef cardNode * cardNodeptr;

---

And I did define cardNode *head as a private data member.

Where is the guarantee that this loop won't attempt to go beyond the end of the list?

Code:

---

                cardNodeptr cur = head;
                cardNodeptr prev = NULL;

                for(int x=1; x<=index; x++)
                {
                        prev = cur;
                        cur = cur->next;
                }

---

What is the purpose of this function? Does it insert new nodes or does it modify the contents of existing nodes? I think you should pick one of those, do that well, and perhaps rename the function to reflect what it does if you decide it shouldn't actually insert nodes.