-2

So I'm having an issue populating this heterogeneous list. I'm passed in a text file and use it to populate the member data of the objects then add them to the list. This is my first time using this and I can't understand why it's not working properly. It should save each populated objects address to a pointer in the array of pointers, correct? However when I test it I can see that it is only saving to the single location of (*list)[0]. The most relevant portion will probably come from main.cpp. Thank you!

main.cpp

#include <iostream>
#include <fstream>
#include <string>
#include <iomanip>
#include "student.h"

using namespace std;

int main()
{
    string inputFileName;
    string outputFileName;

    cout << "Please enter the input file name: ";
    getline(cin, inputFileName);

    ifstream inputFile;
    int NUMBEROFENTRIES;
    inputFile.open(inputFileName.c_str());   //CLOSE
    if(inputFile){
        NUMBEROFENTRIES = int(inputFile.get())-int('0');

    }else{
        cout << "Failed to open file." << endl;
    }

    Student ** list; //create a pointer to Student pointer
    list = new Student*[NUMBEROFENTRIES];  //dynamically allocated list of Student pointers

    for(int i = 0; i < NUMBEROFENTRIES; i++) 
    {
        string uselessNewLine;
        getline(inputFile, uselessNewLine);

        string tempfirstname;
        string templastname;
        getline(inputFile, templastname, ',');
        inputFile.get();
        getline(inputFile, tempfirstname);

        char tempcourse = inputFile.get();
        if(tempcourse == 'b'||tempcourse == 'B')
        {
            BioStudent bobj;

            bobj.setNameAndCourse(tempfirstname, templastname, tempcourse);

            string garbage;
            getline(inputFile, garbage, ' ');

            bobj.SetGrades(inputFile);

            list[i] = &bobj; //I assume this is where the error is but i should be incrementing?

        }

        else if(tempcourse == 't' || tempcourse == 'T')
        {
            TheaterStudent tobj;

            tobj.setNameAndCourse(tempfirstname, templastname, tempcourse);

            string garbage;
            getline(inputFile, garbage, ' ');

            tobj.SetGrades(inputFile);

            list[i] = &tobj;  //I assume this is where the error is but i should be incrementing?


        }

        else if(tempcourse == 'c' || tempcourse == 'C')
        {
            CompsciStudent cobj;

            cobj.setNameAndCourse(tempfirstname, templastname, tempcourse);

            string garbage;
            getline(inputFile, garbage, ' ');
            getline(inputFile, garbage, ' ');

            cobj.SetGrades(inputFile);

            list[i] = &cobj;  //I assume this is where the error is but i should be incrementing?



        }else{
            cout << "ERROR" << endl;
        }

        cout << (*list[0]).course << endl;

    }



    delete [] list;
    return 0;
}

student.h

#include <string>
#include <iostream>
using namespace std;


class Student
{
public:
    virtual double GetAverage()=0;  //Another pure virtual function
    void setNameAndCourse(string fn, string ln, char tc);
    Student();
    char course;
    char GetCourse();
protected:

    string firstName;
    string lastName;


private:

};


class BioStudent: public Student
{
public:
    BioStudent();
    void SetGrades(ifstream &input);
    double GetAverage();

private:
    int labGrade;
    int test1;
    int test2;
    int test3;
    int finalExam;

};


class TheaterStudent: public Student
{
public:
    TheaterStudent();
    void SetGrades(ifstream &input);
    double GetAverage();

private:
    int participation;
    int midterm;
    int finalExam;

};


class CompsciStudent: public Student
{
public:
    CompsciStudent();
    void SetGrades(ifstream &input);
    double GetAverage();

private:
    int assign1;
    int assign2;
    int assign3;
    int assign4;
    int assign5;
    int assign6;
    double assignAverage;
    int test1;
    int test2;
    int finalExam;
};

student.cpp

#include "student.h"
#include <iostream>
#include <iomanip>
#include <string>
#include <fstream>

using namespace std;

Student::Student()
{
    firstName = "";
    lastName = "";
    course = ' ';
}

void Student::setNameAndCourse(string fn, string ln, char tc)
{
    firstName = fn;
    lastName = ln;
    course = tc;

}

char Student::GetCourse()
{
    return course;
}

BioStudent::BioStudent()
{
    labGrade = 0;
    test1 = 0;
    test2 = 0;
    test3 = 0;
    finalExam = 0;
}

void BioStudent::SetGrades(ifstream& input)  
{
    input >> labGrade;
    input >> test1;
    input >> test2;
    input >> test3;
    input >> finalExam;

}

double BioStudent::GetAverage()
{
    double toReturn = 0.0;
    toReturn = ((labGrade*.3) + (test1*.15) + (test2*.15) + (test3*.15) + (finalExam*.25));
    return toReturn;
}

TheaterStudent::TheaterStudent()
{
    participation = 0;
    midterm = 0;
    finalExam = 0;
}

void TheaterStudent::SetGrades(ifstream &input) 
{
    input >> participation;
    input >> midterm;
    input >> finalExam;

}

double TheaterStudent::GetAverage()
{
    double toReturn = 0.0;
    toReturn = ((participation*.4)+(midterm*.25)+(finalExam*.35));
    return toReturn;
}

CompsciStudent::CompsciStudent()
{
    assign1 = 0;
    assign2 = 0;
    assign3 = 0;
    assign4 = 0;
    assign5 = 0;
    assign6 = 0;
    assignAverage = 0.0;
    test1 = 0;
    test2 = 0;
    finalExam = 0;
}

void CompsciStudent::SetGrades(ifstream &input)
{
    input >> assign1;
    input >> assign2;
    input >> assign3;
    input >> assign4;
    input >> assign5;
    input >> assign6;
    input >> test1;
    input >> test2;
    input >> finalExam;

}

double CompsciStudent::GetAverage()
{
    double toReturn = 0.0;
    assignAverage = ((assign1+assign2+assign3+assign4+assign5+assign6)/6);
    toReturn = ((assignAverage*.3)+(test1*.2)+(test2*.2)+(finalExam*.3));
    return toReturn;
}
SWilt
  • 1
  • 1
  • 2
    `list[i] = &bobj` - you just saved the address of an automatic variable that is about to expire, making that address useless. Likewise in every other similar save in this code. I suggest you use a `std::vector>` or similar managed pointer solution. – WhozCraig Jul 19 '16 at 22:17
  • 1
    Unless the exercise is to learn dynamic memory allocation, don't do it. Use a [standard container](http://en.cppreference.com/w/cpp/container) instead (I recommend [`std::vector`](http://en.cppreference.com/w/cpp/container/vector) to begin with). Also, don't store pointers to objects in the container, but actual object instances. Lastly, what you are creating is not a "list" in the computer-science meaning of the word, but rather a dynamically sized *array* of (pointers to) objects. – Some programmer dude Jul 19 '16 at 22:23
  • 1
    "Object instances in the container" will lead to object slicing, if these are polymorphic - which is the case here. Not the case here, but worth to be noted: std::vector can reallocate its internal buffer, so objects might move. If pointers or references to these are necessary, they get invalid then. So we need either a non-reallocating container (e. g. std::list) or we are back at pointers (-> WhozCraig's comment then). – Aconcagua Jul 19 '16 at 23:18
  • (Not to be misunderstood: it is not a bad advice in general, but unsuitable in some special cases.) – Aconcagua Jul 19 '16 at 23:20

1 Answers1

0

You are right, the problem is here:

 list[i] = &bobj; //I assume this is where the error is but i should be incrementing?

and the reason is because bobj is defined on stack as follows:

 BioStudent bobj;

so it will be destroyed once its enclosing scope ends, and then your list will hold a dangling pointer.

What you want is dynamically allocated object:

BioStudent* bobj = new BioStudent;

and then :

list[i] = bobj;

also dont forget to deallocate your objects, or better use smart pointers.

marcinj
  • 48,511
  • 9
  • 79
  • 100
  • Ah you are awesome! I think I'm just confused about dynamic allocation still, I'll watch some videos later tonight. When would be the best time to deallocate them? That's the 'delete bobj' part right? – SWilt Jul 19 '16 at 22:37
  • @SWilt deallocate them right before `delete [] list; Another thing is you need a virtual destructor in your Student class, otherwise you will have Undefined Behaviour - read here http://stackoverflow.com/questions/8599225/virtual-destructor-and-undefined-behavior for more on this. – marcinj Jul 19 '16 at 22:42