Custom class return only returns my ints and not my strings. C++

Question

I am reading in from a file and storing that information to a custom class List which is a linked list of type Node. However when I go to return the list it only returns the ints and drops all of the strings and the vector.

List & importMaster()
{
    fstream file;
    int i = 0, check = 0;
    Node students[10];
    List master1;
    char temp[200];
    file.open("master.txt", ios::in);

    while (!file.eof())
    {

        file.getline(temp,50);
        int recordNum = atoi(temp);
        file.getline(temp, 40);
        int ID = atoi(temp);
        file.getline(temp, 40, '"');
        file.getline(temp, 40, '"');
        const string name = temp;
        file.getline(temp, 40);
        file.getline(temp, 40);
        string email = temp;
        file.getline(temp, 40);
        int units = atoi(temp);
        file.getline(temp, 40);
        string program = temp;
        file.getline(temp, 40);
        const string level = temp;
        file.getline(temp, 40);
        int numAbscence = atoi(temp);
        vector <string> absences;

        for (int j = 0; j < numAbscence; j++)
        {
            file.getline(temp, 40);
            absences.push_back(temp);
        }

        //gets extra line between 
        file.getline(temp, 40);

        students[i] = Node(recordNum, ID, name, email, units, program, level, numAbscence, absences);

        if (i > 0)
        {
            students[i].setNextNode(&students[i - 1]);
        }

        master1.setMaster(&students[i]);

        i++;
    }

    return master1;
}

The condition `!file.eof()` is bad. [c++ - Why is iostream::eof inside a loop condition considered wrong? - Stack Overflow](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong) — MikeCAT, Jul 14 '16 at 04:47
Please post a [Minimal, Complete, and Verifiable example](http://stackoverflow.com/help/mcve) with required declarations and definitions. — MikeCAT, Jul 14 '16 at 04:48
Will definitely need to know what `setMaster` is doing. It looks like it may just be storing a pointer to a local variable that will go out of scope, but we don't know — Tas, Jul 14 '16 at 05:25

score 0 · Answer 1 · answered Jul 14 '16 at 06:18

You're returning a reference (List&) to master which is a List on the stack - this is bad because master goes out of scope as soon as you return, and the part of the stack that master lived in will likely soon be used for another local variable.

You could change it to just return List instead of List&, and hope that the compiler optimizes out the copy (via RVO = Return Value Optimization). Alternatively you could explicitly do what RVO essentially does and pass in a reference, e.g:

void importMaster(List& outMaster)
{
    // same as before, just remove the "List master1;" line
    // and replace-all master1 with outMaster
}

and then do:

List masterList;
importMaster(masterList)

However, even then I suspect you might have issues with these lines:

students[i].setNextNode(&students[i - 1]);

and master1.setMaster(&students[i]);

It's impossible to tell without seeing the implementations (do they hang onto the pointer passed in, or do they copy the object) - but if they just hang onto the pointers then they will become invalid as soon as students goes out of scope (i.e. when you return from that function).

You're also not doing any bounds checking so if i ever gets >=10 then you're gonna start stomping other stuff on the stack

Custom class return only returns my ints and not my strings. C++

1 Answers1