Adding to a vector from a function

Ok, I have the following code:

Code:

void student::add_assignment(assignment *hw)
{
	std::cout<<"Size of assignment vector right now:"<<this->homework.size()<<"\n";
	this->homework.push_back(*hw);
	std::cout<<"Size of assignment vector now:";
	std::cout<<this->homework.size()<<"\n";
}

Now then, this code should (from my test application) print out zero, add the assignment passed to the function to a vector of assignments, and then prinout the size again. This works great.
However, in another call to homework.size(), which is called directly after the above code is called, homework is reported as being of size 0 (that is, homework.size() returns 0), and thus creates an abnormal program termination after trying to return the assignment at the position passed to the function.
Here is that code:

Code:

assignment student::get_assignment(const int a)
{
	std::cout<<"Size of assignment vector is:";
	std::cout<<this->homework.size();
	std::cout<<"\n";
	assignment *temp = new assignment(0,true,'a',22);
	if(a > homework.size() || homework.empty())
	{
		return *temp;
	}
	else
	{
		return homework.at(a);
	}
}

In other words: at the end of the first function, the vector homework is of size 1 (that is, it has one element in it). However, at the beginning of the second function, it is reported to be of size 0, and thus always returns the temp assignment created in it.

Basicly, I want to know why the first function doesn't add the element to the vector for the variable that holds the vector.

Here is how the code is called:

Code:

//From main.cpp
#include <cstdlib>
#include <iostream>
#include "course.h"
#include "student.h"
#include "assignment.h"

using namespace std;
course *asdf = new course("asdf", 1, "me");



int main(int argc, char *argv[])
{
	student *xyz = new student();
	asdf->add_students(xyz);
	asdf->get_student(0).add_assignment(new assignment(90505, true, 'a', 0));
	long g = asdf->get_student(0).get_assignment(0).get_duedate();
	std::cout<<g;
	std::cin>>g;
    return 0;
}

Any ideas? If you need any more of the code just ask for it.

Are you returning the student by reference from get_student(0)? If not, then the assignment is being added to a temporary student which is probably your problem.

On another note, you should not be using new anywhere in your program. You are leaking memory everywhere. C++ is different than Java. You should just use the constructor of the object to create a temporary value to pass to the vector method which will make its own copy. From the code shown I don't think you need pointers at all.

Originally Posted by Daved

Are you returning the student by reference from get_student(0)? If not, then the assignment is being added to a temporary student which is probably your problem.

On another note, you should not be using new anywhere in your program. You are leaking memory everywhere. C++ is different than Java. You should just use the constructor of the object to create a temporary value to pass to the vector method which will make its own copy. From the code shown I don't think you need pointers at all.

Wow...I can't believe that...

It worked, but I'm amazed that after three days of working on this issue I didn't even think of returning a reference. Thanks!

About your second paragraph, can you show an example of what you mean? I'm a bit lost by your wording

First, your assignment and student classes (and any other class you use) should have valid copy constructors and copy assignment operators. Whether you need to write them yourself or the default ones are correct depends on the data you are holding. If you are holding strings, vectors, POD types (like int, double, or bool) only, then you don't need to add your own. If you have allocated memory using new, or arrays like C style strings, then you need to make your own.

Second, assuming your classes have valid copying, you shouldn't need pointers at all. You pass objects around as reference parameters. Your store them as objects in the vectors. Here is my example:

Code:

void student::add_assignment(const assignment& hw)
{
   homework.push_back(hw);
}

assignment& student::get_assignment(unsigned int a)
{
   // std::out_of_range exception automatically thrown by at() if a is not valid.
   return homework.at(a);
}

int main()
{
   course asdf("asdf", 1, "me");
   student xyz;
   asdf.add_students(xyz);
   asdf.get_student(0).add_assignment(assignment(90505, true, 'a', 0));
   long g = asdf.get_student(0).get_assignment(0).get_duedate();
   std::cout<<g;
   std::cin>>g;
   return 0;
}

Notice how in main, asfd is declared locally. It will be cleaned up automatically when the function ends (in this case at the end of the program since the function is main()). Your old way you used new but not delete, so the memory was never cleaned up and the destructor for course was never called.

The same thing is true for xyz. Declare it locally so that it will clean itself up automatically. There is no reason to use new. You rarely need to use new in C++ unless you are using dynamic polymorphism (via inheritance).

Notice how xyz was added as a student (assuming you changed add_students to take a reference parameter instead of a pointer). You can pass a local variable and a copy will be added to the vector.

You can also pass a temporary variable, which is what happens when the add_assignment function is called inside main. Instead of creating a named variable like xyz, I just used the assignment constructor to create a temporary variable with those arguments (90505, true, 'a', 0) and passed it to the add_assignment function. That function added a copy of that into the vector.

In none of these places is new required. If for some reason you do use new, you must remember to save a copy of the pointer somewhere until you are done with the object and then clean it up yourself by calling delete. There is no garbage collection, so you must clean the dynamically allocated objects up yourself. That is why it is usually better to just create local objects that are destroyed automatically.