1

I have a class named Student

class Student
{   string name;
    unsigned long int ID ;
    string email;
    unsigned short int year;
    public : 
         Student() // Constructor
         string getName(void);
         unsigned long int getID(void);
         string getEmail(void);
         unsigned short int getYear(void);   
{

and another class named eClass

class eClass {
 private:
 string eclass_name;
  Student* students[100];
  unsigned int student_count;

   public:
    eClass(string name)
    {
        student_count  =0 ; 
        eclass_name = name  ; 
    }

        bool exists(Student obj)
    {
        unsigned long int code = obj.getID();
        bool flag = TRUE ;
        for (unsigned int i = 0 ; i<=student_count ; i++ )
        {
            unsigned long int st = (*students[i]).getID();
            if (code==st)
            {
                flag = FALSE;
            }
        }
        return flag;
    }


        void add(Student& obj)
    { 
        bool res = exists(obj);
        if (res)
        {
            students[student_count] = new Student(); //probably the problem is here
            *students[student_count] = obj ;  
            student_count++ ; 
        }
    }

    string getEclassName(void) { return eclass_name; }
    unsigned int getStudentCount(void) { return student_count; }
    Student getStudent(int i) { return *students[i-1]; }


     };    

The statement Student* students[100]; must look exactly like this . For example I can't write something like this: Student* students[100]={} ;

And main() looks like this

    int main()
   {
     Student    JohnDoe("John Doe", 12345,  2,  "[email protected]");
     eClass Cpp("C++"); 
     Cpp.add(JohnDoe);


     }

Basically I have an array of pointers to Student objects and I want to allocate dynamically memory every time I want to add a new Student object.

When I compile I get no errors but when I try to run the program the only thing I get is "Program_name.exe" stopped running...

I'm pretty sure the problem has to do with memory allocation but I'm not able to find it and solve it.

Any suggestions ?

8
  • from what I can see, the code above looks fine; can you post the implementation of Student? Commented Dec 10, 2015 at 15:09
  • Why are you using pointers to Students? Just store Students. Commented Dec 10, 2015 at 15:09
  • From what you posted, I estimate the major bug is in exists(Student obj). Anyway, you shouldn't be passing by value into that function (but I don't think that is the major bug). Add your implementation of exists to you post. Commented Dec 10, 2015 at 15:12
  • 1
    The prime suspect is exists. (Side note: exists returning true if the student doesn't exist is very counter-intuitive.) Commented Dec 10, 2015 at 15:13
  • I edited and added the Student class and the exist function... Using pointers isn't my choice...Our professor asked us to do it this way Commented Dec 10, 2015 at 15:15

2 Answers 2

2

The main bug in exists was the loop went one too far, using an uninitialized pointer. But also it is very bad style for exists to take its input by value. Fixing both of those:

bool exists(Student const& obj)
{
    unsigned long int code = obj.getID();
    bool flag = TRUE ;
    for (unsigned int i = 0 ; i<student_count ; i++ )
    {
        unsigned long int st = (*students[i]).getID();
        if (code==st)
        {
            flag = FALSE;
        }
    }
    return flag;
}

You should declare getID() const inside student in order to be able to code exists correctly.

unsigned long int getID() const;
Sign up to request clarification or add additional context in comments.

1 Comment

Wow...thanks a lot my friend...I never imagined that one more loop would create such a big problem!!!
0

First, you should initialize all of your student pointers to either NULL or nullprt. This is not strictly needed but is a very good habit to get into. You'll thank yourself later.

Second, why are you returning false if the student exists? Kind of confusing I'd imagine. Also, you can use the break statement after you find your student exists; no need to check the rest of them.

Also, on your add, you may want to check to ensure you don't have MORE than 100 students. This will overwrite memory and bad things will happen.

4 Comments

Notice the use of student_count to control the use of students. So if student_count were used correctly, there is no need to initialize students and when student_count was used incorrectly, initializing students would not have helped.
@JSF Added an explanation on that one. I know it isn't NEEDED but it is a good habit to get into this early on. Always initialize EVERYTHING; pointers, ints, classes, conversation, etc ....
I wouldn't have commented if your second sentence had been there when I commented. BUT the word "need" in the first sentence is still wrong (contradicted by the second). "ought to" or "should" would cover it.
I was changing that as you commented. I thought I'd removed that first need. Let me get that.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.