pointer problem with vector

[Claymz]

Hi,

i have this container of pointers of base class, so i can get dynamic type of derivaed case (which i put in this vector container). The problems strucks, when i try to add another derived class, where all the previous derived classes which are the same and are already in the vector (elements of vector) get the same as the one just entered. It has something to do with pointers, i tried to declare subclasses where i needed them, but as it seems, if i put it outside the main while loop, only the values of non-base class get filled, and even those not correctly (zeros mainly). So im asking if someone could tell me what should i change or merely transfer, to make it work right; also why cant declarations be made in case statements?

the code...

int main()
{
	vector<Employee*> database;
	bool quit = false;
	int choise = 0;
	int choise1 = 0;
	float defFloat = 0.0;
	int defInt = 0;
	string defString = "";
	bool defBool = false;
	Manager manager;
	Engineer engineer;
	Researcher researcher;

	while(!quit)
	{
		cout << "1.Add an employee, 2.Delete an Employee, 3.Save database, 4.Exit" << endl;
		cin >> choise;

		switch(choise)
		{
		case 1:
			{
			cout << "1.Manager, 2.Engineer, 3.Researcher" << endl;
			cin >> choise1; 

			switch(choise1)
			{
			case 1:	
				{
					cout << "Enter first name: ";
					cin >> defString;
					manager.setFname(defString);
					cout << "Enter last name: ";
					cin >> defString;
					manager.setLname(defString);
					cout << "Enter salary: ";
					cin >> defFloat;
					manager.setSalary(defFloat);
					cout << "Enter number of meetings per week: ";
					cin >> defInt;
					manager.setNumOfMeetingPerWeek(defInt);
					cout << "Enter number of vacation days per year: ";
					cin >> defInt;
					manager.setNumOfVacationDaysPerYear(defInt);
					database.push_back(&manager);
					break;
				}
			case 2:
				{
					cout << "Enter first name: ";
					cin >> defString;
					engineer.setFname(defString);
					cout << "Enter last name: ";
					cin >> defString;
					engineer.setLname(defString);
					cout << "Enter salary: ";
					cin >> defFloat;
					engineer.setSalary(defFloat);
					cout << "Enter knowladge of C++(1-knows C++, 0-doesnt know C++) : ";
					cin >> defBool;
					engineer.setCpp(defBool);
					cout << "Enter number of years of experiance: ";
					cin >> defInt;
					engineer.setExp(defInt);
					cout << "Enter type of engineer: ";
					cin >> defString;
					engineer.setType(defString);
					database.push_back(&engineer);
					break;
				}
			case 3:
				{
					cout << "Enter first name: ";
					cin >> defString;
					researcher.setFname(defString);
					cout << "Enter last name: ";
					cin >> defString;
					researcher.setLname(defString);
					cout << "Enter salary: ";
					cin >> defFloat;
					researcher.setSalary(defFloat);
					cout << "Enter PhD scholl: ";
					cin >> defString;
					researcher.setPhDScholl(defString);
					cout << "Enter PhD thesis topic: ";
					cin >> defString;
					researcher.setPhDThesisTopic(defString);
					database.push_back(&researcher);
					break;
				}
			}
			break;
		}
		case 2:
			{
				cout << "Enter Employee's last name: " << endl;
				cin >> defString;

				for(int i = 0; i < database.size(); i++)
				{
					if(!database.at(i)->getLname().compare(defString))
					{
						database.erase(database.begin() + i);
						break;
					}
				}
				break;
			}
		case 3:
			{
				ofstream outFile("database.txt");
				if(outFile)
				{
					for(int i = 0; i < database.size(); i++)
					{
						database.at(i)->save(outFile);
					}
					outFile.close();
				}
				break;
			}
		case 4:
			{
				quit = true;
				break;
			}
		}
	}
}

[Disch]

Okay... pointers do not give you a new object. They tell the computer where to find an existing object.

Here's a very simple example:

int myvar = 0;
int* myptr = &myvar;  // myptr "points to" myvar

*myptr = 5;  // change whatever myptr points to
  // ie:  this changes myvar

cout << myvar;  // prints "5" 

As you can see in this example, since myptr points to myvar, when we change *myptr we are effectively changing myvar. The inverse is also true:

myvar = 10;
cout << *myptr;  // prints "10" 

So how does this apply to your problem?

Well very simply you only have 3 objects:

	Manager manager;
	Engineer engineer;
	Researcher researcher;

You keep adding the same pointers to these objects over and over. Each time you do that, you are not creating a new object with new details -- you're simply changing the old object and duplicating a pointer to it.

So you're basically doing this:

Manager manager;

Manager* ptra = &manager;
ptra->whatever = whatever;  // this changes the 'manager' object

Manager* ptrb = &manager;
ptrb->whatever = whatever;  // this changes the 'manager' object AGAIN
  // which of course ALSO changes *ptra because ptra still points to manager. 

So what's the solution?

Dynamic allocation! Instead of having those 3 fixed objects, dynamically allocate new objects when you need them:

case 1:
{
  Manager* manager = new Manager;  // create a new manger

  // set manager stuff

  database.push_back( manager );
}
break;

(Of course if you do this, then you'll need to delete all the pointers when you empty the list or else you'll leak memory)

[DSLq5Di1]

Consider using a unique_ptr to manage your objects too,

#include <memory>
#include <utility> 

vector< std::unique_ptr<Employee> > database;
...

  case 1:
  {
    std::unique_ptr<Manager> manager(new Manager);
  
    // set manager stuff

    database.push_back(std::move(manager));
  }

Now when you erase an object from your vector, memory will be cleaned up for you.

[Disch]

Yes that is definitely better, assuming you can use C++11

[Claymz]

Thank you very much, it works now. I just have one more quastion - how should i delete allocated memory in my program the "old fashioned way" with delete operator, not like sloppy9 posted (just for learning purposes)?

[Disch]

When you're done with the vector, do this:

for(unsigned i = 0; i < database.size(); ++i)
  delete database[i];

database.clear();

Also note that your parent class (Employee) must have a virtual destructor because you are cleaning up polymorphically.

[Claymz]

1. Why is it needed to make a loop and delete every element of an array seperately, when you can do it all at once with delete database[] - its because this way, you delete the actual/dynamic type of object, instead of static type, that is Employee for each object, which would be used if we used delete database[] right?(this way we delete all the objects the right way - with the most derived types; we would actually delete allocated memory of database array/vector, which has allocated none)?

2. Does database.clear() have the same meaning as = 0 for arrays and variables after deleting the allocated memory, because vector works with dynamic memory also, and with these we explicitly set/check, that it does not point to any pointer whatsoever(in both examples - with clear() and = 0)? And why isnt it used always - do you use it only when your not sure if it still points to something right? Otherwise, if your sure that it points to nothing you can skip the clear()/= 0 part. That would mean that the underlined code is not neccessary(not sure for clear() though, need to hear from you):

for(unsigned i = 0; i < database.size(); ++i)
{
  delete database[i];
  database = 0;
}

database.clear();

3. And for destructor, i just added this:

//decleration
//Employee class
virtual ~Employee();

//implementation
//Employee class
Employee::~Employee() {}

4. why do i get a warning: warning "C4018: '<' : signed/unsigned mismatch", when im not using un/signed in a for loops? And if i use only the un/signed keyword, which type is actually used, what does it actually mean? I know what un/signed int means, for example, but not what just un/signed means.

5. if you need types with dynamic memory the whole time your program is running, is it even neccessary to do delete allocated memory, since i presume when the program quits, it will no longer have to deal will memory, and therefore are the memory occupied by the program will be released, and therefore no memory release would be needed? (the same with clear() for vector)

[Disch]

Why is it needed to make a loop and delete every element of an array seperately, when you can do it all at once with delete database[]

delete and delete[] do two different things.

delete takes one pointer which points to one object and deletes that one object.

delete[] takes one pointer which points to several objects (stored one after the other in memory) and deletes them all.

The reason you can't use delete[] here is because the objects are not stored consecutively in memory. Their pointers might be stored consecutively in the vector, but the objects themselves are not. Remember we have multiple pointers here, but delete[] only works with a single pointer.


But really the short and easy answer is because you had multiple new's. Each new must have a matching delete, and each new[] must have a matching delete[]. Since you used new in a loop to allocate the objects, you must also use delete in a loop to free them.

Does database.clear() have the same meaning as = 0 for arrays and variables after deleting the allocated memory,

Kinda sorta. clear() makes the pointers "go away" so they don't exist any more. It doesn't matter whether or not they're zero'd because their memory is freed and they no longer exist.

It's kind of like this:

void func()
{
  int myvar;
  // do a bunch of stuff with myvar

  myvar = 0;  // set myvar to 0 before the function exists because we're done with it
} // myvar "goes away" here 

Zeroing myvar here is pointless because very shortly, myvar isn't going to exist. So it doesn't matter what its contents are.

It's the same thing with your vector:

for(unsigned i = 0; i < database.size(); ++i)
{
  delete database[i];
  database[i] = 0;  // <-  zeroing individual pointers here doesn't really do anything
}

database.clear(); // <- because once you clear, all those pointers no longer exist 

The delete line, on the other hand, is important because that does not delete the pointer, it deletes the object that the pointer points to (which is not contained in the vector, and thus doesn't go away when the vector is cleared).

I really need to get around to finishing that pointer tutorial I was working on.


Furthermore, the vector will be automatically cleared in its destructor. So you don't need to clear the vector if it will be going out of scope.

In both cases, though (pointer = 0 and database.clear()), it really doesn't hurt anything to do them. You could say it adds clarity to your code. So it isn't a BAD thing to do, it just isn't really necessary.

3. And for destructor, i just added this:

That's perfect. That's all you need.

why do i get a warning: warning "C4018: '<' : signed/unsigned mismatch", when im not using un/signed in a for loops?

vector::size returns an unsigned variable (type: size_t). int is signed. Comparing a signed and unsigned type gives you that warning on some compilers. If you want to avoid it, make i a size_t instead of an int.

I know what un/signed int means, for example, but not what just un/signed means.

Signed variables' most significant bit have a negative weight. In english, this means that they can represent signed numbers, but in doing so they sacrifice the ability to represent higher numbers.

Unsigned variables can't represent signed numbers, but can represent higher numbers than their signed counterparts.

range of signed char: -128 .. 127
range of unsigned char: 0 .. 255
range of signed short: -32768 .. 32767
range of unsigned short: 0 .. 65536
etc

if you need types with dynamic memory the whole time your program is running, is it even neccessary to do delete allocated memory, since i presume when the program quits, it will no longer have to deal will memory, and therefore are the memory occupied by the program will be released, and therefore no memory release would be needed?

You are correct. Memory will all be automatically freed en masse by the OS once your program exits. So if you are sure the lifetime of those objects will last the entire program then you don't HAVE to delete them. Some people would consider that bad practice though.

One side-effect of this is that the object's destructors will not run. So if you have the destructor doing something that would be outwardly visible (like generating a log file or something), that won't happen unless you actually delete the object.

(the same with clear() for vector)

As previously mentioned, you never have to call clear() as part of cleanup. vector will do it automatically when it is destroyed.

[Claymz]

about signed/unsigned types, i meant like so: signed i or unsigned i. What does this mean, which actual type is being signed/unsigned in this case?