2

I'm writing code to store exam results for different degrees (physics & chemistry currently).

I have an abstract base class student which is as follows:

class student{
public:
    virtual ~student(){}
    //virtual function to add course + mark
    virtual void addresult(std::string cName,int cCode,double cMark)=0;
    //virtual function to print data
    virtual void printinfo()=0;     //to screen
    virtual void outputinfo(std::string outputfilename)=0;      //to file
};

I then have a derived class for physics (and will have a similar one for chemistry):

class physics : public student{
protected:
    std::string studentname;
    int studentID;
    std::map<int,course> courses; //map to store course marks
public:
    //constructors/destructors
    void addresult(std::string cName,int cCode,double cMark){course temp(cName,cCode,cMark);courses[cCode]= temp;}

    void printinfo(){
        //function to output information to screen      
    }

    void outputinfo(std::string outputfilename){
        //function to write student profile to file
    }
};    

In my main, I would then like to have a map that can store all students within it (physics and chemistry). The student ID as the key with a base class pointer to either a physics or chem. student is, I'm guessing, the way to go.

I tried the following code:

map<int,student*> allstudents;
physics S1(sName,sID);
physics* S2 = &S1;
allstudents.insert(pair<int,student*>(sID,S2));

but this didn't work. I think I'm getting slightly confused with what should be pointing to what. Can you even do this with maps? Should there also be any 'clearing up' required if I store the info. this way? Thanks for your help.

QuantumO
  • 187
  • 2
  • 12
  • 3
    Public inheritance models an IS-A relationship, but physics is a science, not a student. – sbi Apr 15 '12 at 10:05

2 Answers2

3

You can, but you're missing a couple of things:

  • you're not respecting the rule of three. You need to also define the assignment operator and copy constructor in your classes.

  • you're possibly encountering memory corruption issues:

The following

physics S1(sName,sID);
physics* S2 = &S1;
allstudents.insert(pair<int,student*>(sID,S2));

will insert a pointer that becomes dangling when S1 goes out of scope. You should either use a smart pointer or delegate memory management to the map - i.e. create your object with new and delete it when the map goes out of scope.

Luchian Grigore
  • 253,575
  • 64
  • 457
  • 625
3

If you use a pointer for more than a mere second, you should not create the object on the stack and then point to it! It will be gone as soon as the next } shows up, and your pointer will be invalid!

Use physics* S2 = new physics(sName, sID); instead. Use delete on all pointers in your map (iterator comes handy here) to clean up!

ypnos
  • 50,202
  • 14
  • 95
  • 141
  • 1
    Or use [smart pointers](http://stackoverflow.com/questions/106508/what-is-a-smart-pointer-and-when-should-i-use-one) so you don't have any headaches from manual memory management and exception-unsafety… –  Apr 15 '12 at 10:11
  • I have changed to: physics* S1= new physics(sName,sID); allstudents.insert(pair(sID,S1)); but still get the error: "c:\program files\microsoft visual studio 10.0\vc\include\utility(163): error C2440: 'initializing' : cannot convert from 'physics' to 'student *'" Any ideas? – QuantumO Apr 15 '12 at 12:50
  • Looks like you are missing the * somewhere. If you don't spot the error, ask a new question. – ypnos Apr 16 '12 at 11:35
  • It seems the errors produced when I used just a pointer to the class (instead of pointer to abstract base class) appeared to be more useful. I should probably learn to walk before I try and run next time! But thanks for your help. – QuantumO Apr 17 '12 at 19:44