getting data value from node, pointer confusion

Question

So because of a circular dependency between my classes my Node looks like

struct Node{
    Word * data;
    Node * next;
};

So when creating a new Node I do (w being a Word type, newptr being a data member of the class)

newptr = new Node;
newptr-> data = &w;
newptr-> next = NULL;

If I want to access the data in the first node of a linked list, I think the correct method is

Word retVal;
Node *temp;
temp = head->next;
retVal = temp->data;
return retVal;

However data is a pointer to a Word type, it is not a Word type itself. But I want to return a Word, not a Word pointer. So would I do could I add Word * word; then word = temp->data; and retVal = *word;? My test program tells me something I did along the way is wrong because I'm getting a segmentation fault when I reach the function. I'm trying this

Word retVal;
Word * word;
Node *temp;
temp = head->next;
word = temp->data;
retVal = *word;
return retVal;

And using a cout line at various places I know my fault occurs at the word = temp->data line

My Word class members as requested:

class Word{
    public:
        char * charArray;
        char * sendBack; //ignore
        char * rest; //ignore
        bool isPigLatin;
        bool firstIsVowel;
        Word();
        Word(const Word& w);
        Word(char array[], int size);
        ~Word();
        void show(); //ignore
        //ignore everything below
        friend ostream& operator<< (ostream& out, const Word& w);
        Sentence operator+ (Sentence s);
        Sentence operator+ (Word w);
        void operator+ (int i);
        Word& operator++(int);
        Word& operator++();
        Word& operator--(int);
        Word& operator--();
};

A lot of these things don't have to do with the linked list itself but I included everything just in case

Copy constructor for Word, as requested:

Word::Word(const Word& w){
    cout << "Copy constructor for Word" << endl;
    int size = 0;
    while(w.charArray[size]){
        size++;
    }
    charArray = new char[size + 1]; //+1 for \0 at end
    int i = 0;
    while(w.charArray[i]){
        charArray[i] = w.charArray[i];
        i++;
    }
    if(!w.isPigLatin){
        isPigLatin = false;
    }
    else{
        isPigLatin = true;
    }   
}

firstIsVowel, sendBack, and rest aren't declared until they need to be used in the ++ and -- overloads as they are for making a word into Pig Latin and back into English

my constructor:

    Word::Word(char array[], int size){
//In my test code I am hard coding a char array and its size, for the actual program I read in the char array from a file, then go thru the char array to get the size before calling Word(char c[], int s)
        cout << "Character Array Word constructor" << endl;
        int i = 0;
        charArray = new char[size];
        while(array[i]){
            charArray[i] = array[i];
            i++;
        }
        isPigLatin = false;
//this is needed for my Pig Latin function. All words will be read in as English, thus Piglatin = false. If the function to change PigLatin into English is called, nothing will happen since this is false. It gets set to true in the function that changes English to Pig Latin
    }

and default constructor

Word::Word(){
    cout << "Default constructor for Word" << endl;
    charArray = new char[1];
    sentBack = new char[1];
    rest = new char[1];

}

Answer 1

You stated that you wanted to return a Word object. Given that the object contains pointers to dynamically allocated memory, it needs to be copied correctly.

A correct copy means that the copy must be indistinguishable from the original object. For your situation, this means that all the members from the source object need to be assigned to the destination object. Your code failed to do that for two pointer members (sendBack and rest) and for one bool firstIsVowel.

Let's throw out the two pointer variables and assume Word doesn't have them. The copy constructor would look something like this:

Word::Word(const Word& w) : isPigLatin(w.isPigLatin), firstIsVowel(w.firstIsVowel)
{
    int size = strlen(w.charArray);
    charArray = new char[size + 1]; 
    strcpy(charArray, w.charArray);
}

Note that I copied all the members directly from the passed in object. No "business logic", no fooling around checking what the values were for those bools, nothing. Just copied. That is the job and only job of a copy constructor.

Second, you need an assignment operator. The signature for the assignment operator is as follows:

Word& operator = (const Word& w);

In other words, you need to be able to assign a Word to another Word. That can be written like this:

Word& Word::operator=(const Word& w)
{
    Word temp(w);
    std::swap(isPigLatin, temp.isPigLatin);
    std::swap(firstIsVowel, temp.firstIsVowel);
    std::swap(charArray, temp.charArray);
    return *this;
}

This uses the copy/swap idiom, described in many links on SO. I won't go into it, but it does require that you have a working copy constructor and destructor for Word to implement it. The std::swap is a utility function that swaps the two values.

So this should take care of the copying of Word, if we remove the extraneous two char pointers. If they must be in there, then you need to amend the code above to correctly allocate and copy the data for those two pointers.

Answer 2

Look at what each line of code is doing on its own:

First, you are allocating a Node object on the heap and storing its address-value in a pointer-variable called 'temp'

temp = new Node;

Second, you are leaking that memory by overwriting its address-value, replacing that address-value with the one contained in the 'head' pointer-variable.

temp = head;

Thirdly, you're overwriting the address-value in 'temp' again by storing the address-of the second item in your linked list

temp = temp->next;

In fact, you could have replaced those 3 lines with the following:

temp = head->next;

Already, you have the potential for an error here, because there's no guarantee that head is not null, so you could improve the code by checking it against null.

Next, when you call delete on the temp variable, the second node in your list is released; leaving the first node in your list with a "dangling" (invalid) value for its next pointer.

However, since all you're after is the first node, (and assuming you want to delete the first node, not the second), chances are you probably wanted to do this instead:

Word* retVal = nullptr;

if (head != nullptr)
{
    // (1) Remember the address-of the first node for later deletion
    Node* temp = head;

    // (2) The second node becomes the new head
    head = head->next;

    // (3) Grab the data from the old head
    retVal = temp->data;

    // (4) Release the memory for the old head
    delete temp;
}

return retVal;