new to vectors, problems

[paradoxresolved]

Hi all,

I'm experimenting with vectors and trying to accomplish some basic things:

1) trying to figure out whether or not "new" objects are deleted when the vector is destroyed or cleared.

2) Saving and loading a vector by unformatted means.

The following program should produce:

0
0
1
2
3
4

but it doesn't. First, it appears that list.clear() is not working. So let's start with that problem. Can anyone tell me why list.clear() is not clearing the vector? Thank you.


#include <vector>
#include <iostream>
#include <conio.h>
#include <fstream>
using namespace std;

class MyClass
{
public:
MyClass(int value);
virtual ~MyClass();

int Value;

};

MyClass::MyClass(int value)
{
Value = value;
}

MyClass::~MyClass()
{

}

int main()
{
int i = 0;
vector<MyClass> list;
MyClass* pNewMyClass = NULL;

for( i = 0 ; i < 5 ; i++)
{
pNewMyClass = new MyClass(i);
list.push_back(*pNewMyClass);
}

ofstream out("C:/CppPrograms/VectorExperiment/TempFile", ios::out | ios::binary );

if(!out)
{
cout << "Cannot open output file.\n";
return 1;
}

out.write((char*) &list, sizeof(class vector<MyClass>));
out.close();

list.clear();

int size = list.size();
for( i = 0 ; i < size ; i++ )
{
cout << list[i].Value << endl;
}

ifstream in("C:/CppPrograms/VectorExperiment/TempFile", ios::out | ios::binary );

if(!in)
{
cout << "Cannot open input file.\n";
return 1;
}
in.read((char*) &list, sizeof(class vector<MyClass>));

for( i = 0 ; i < 5 ; i++)
{
cout << list[i].Value << endl;
}
in.close();

_getch();
return 0;
}

[Calculator]

Surely you are enjoying yourself? Testing section maybe is more suitable?
Edit: forgiven ;) I just saw it sitting there for a minute with some blank stuff

[paradoxresolved]

Sorry Calculator. . . I was trying to figure out how to format my posting. I did not mean for that to be posted as it was. If a moderator is reading, please change the title to "new to vectors, problems". Thank you, and I apologize for this trouble. I'm being rushed on someone elses computer. :(

[Paul McKenzie]

Hi all,

I'm experimenting with vectors and trying to accomplish some basic things:

1) trying to figure out whether or not "new" objects are deleted when the vector is destroyed or cleared.
No. The vector has no idea that the objects being placed in it were created with operator new by you. Therefore it cannot call "delete". The basic rule is that if you call "new" or "new[]", then you have to call "delete" or "delete[]".

Second, I looked at your code. To make it easier, create a default constructor so that you can store objects, not pointers. Your current code has a memory leak, since you never called "delete" on those objects. Instead, you could just have done this:

Third, do not call your variable "list". There is already a std::list class for C++, and your code will cause confusion, and in some cases, compiler errors.

Fourth, why are you testing your program's output by doing something like writing to a file? Why not just do a simple "cout" to test things? Right now, you don't know if the problem is caused by vector, or you not doing the output correctly.

Fifth, a std::vector is non-POD (Plain-Old-Data). You cannot use functions such as iostream::write() to output the vector's contents, or read() to read in values. This is C++, not 'C'. To input/output values of non-POD classes, you must do something called serialization. The easiest form of this is to write out or read in the values one-by-one in a loop.

In general, functions that act on raw memory blocks, i.e. memset(), memcpy(), malloc(), fwrite(), etc. cannot be used on non-POD C++ classes and structs. They do work on POD (C-compatible) structures, i.e. structs or classes that contain only ints, doubles, chars, arrays, etc. Read up on the many threads that discuss this here.

Here is a rewrite of your code:

#include <vector>
#include <iostream>
#include <fstream>

class MyClass
{
public:
MyClass(int value = 0);
int Value;
};

MyClass::MyClass(int value) : Value(value)
{ }

using namespace std;

int main()
{
int i = 0;
vector<MyClass> Mylist;

for( i = 0 ; i < 5 ; i++)
Mylist.push_back( MyClass(i));

ofstream out("C:/TempFile", ios::out | ios::binary );
if(!out)
{
cout << "Cannot open output file.\n";
return 1;
}

// This is totally wrong!
// out.write((char*) &list, sizeof(class vector<MyClass>));
for (int i = 0; i < Mylist.size(); ++i )
{
out << Mylist[i].Value << "\n";
cout << Mylist[i].Value << "\n";
}
Mylist.clear();

int size = Mylist.size();
for( i = 0 ; i < size ; i++ )
{
cout << Mylist[i].Value << endl;
}
}
I didn't do the reading in, since that is wrong also, similar to the way you were doing the output. Hopefully you see how to fix it.

Regards,

Paul McKenzie

[paradoxresolved]

I'm a little confused at this point. . . I've rewritten my code (before having a chance to read your response). It seems like vector is actually deleting the objects. . . Here is the code:


#include <vector>
#include <iostream>
#include <conio.h>
#include <fstream>
using namespace std;

class MyClass
{
public:
MyClass(int value);
virtual ~MyClass();

int Value;

};

MyClass::MyClass(int value)
{
Value = value;
}

MyClass::~MyClass()
{
cout << "MyClass destroyed" << endl;
}

int main()
{
int i = 0;
vector<MyClass> list;
MyClass* pNewMyClass = NULL;

cout << "Filling list" << endl;
for( i = 0 ; i < 5 ; i++)
{
pNewMyClass = new MyClass(i);
list.push_back(*pNewMyClass);
}
cout << "List is full" << endl;

ofstream out("C:/CppPrograms/VectorExperiment/TempFile", ios::out | ios::binary );

if(!out)
{
cout << "Cannot open output file.\n";
return 1;
}
else
{
cout << "File opened for saving" << endl;
}
cout << "Begin saving" << endl;
out.write((char*) &list, sizeof(class vector<MyClass>));
cout << "Saving complete" << endl;
out.close();

cout << "File closed" << endl;

list.clear();

cout << "List cleared" << endl;

int size = list.size();
cout << "size of list is: " << size << endl;
for( i = 0 ; i < size ; i++ )
{
cout << list[i].Value << endl;
}
_getch();

ifstream in("C:/CppPrograms/VectorExperiment/TempFile", ios::out | ios::binary );
if(!in)
{
cout << "Cannot open input file.\n";
return 1;
}
else
{
cout << "File opened for loading" << endl;
}

cout << "Now loading from file" << endl;
in.read((char*) &list, sizeof(class vector<MyClass>));
cout << "Load complete. yippee." << endl;

for( i = 0 ; i < 5 ; i++)
{
cout << list[i].Value << endl;
}
in.close();
cout << "Closing file after loading." << endl;

_getch();
return 0;
}


. . . and here are the results:

Filling list
MyClass destroyed
MyClass destroyed
MyClass destroyed
MyClass destroyed
MyClass destroyed
MyClass destroyed
MyClass destroyed
List is full
File opened for saving
Beging saving
Saving complete
File closed
MyClass destroyed
MyClass destroyed
MyClass destroyed
Myclass destroyed
Myclass destroyed
list cleared
size of list is: 0
<pause for user to press a key>
File opened for loading
Now loading from file
Load complete. yippee.
0
1
2
3
4
Closing file after loading.
<pause for user to press a key>
MyClass destroyed
MyClass destroyed
MyClass destroyed
MyClass destroyed
MyClass destroyed


I don't understand what is happening here. Why is the MyClass destructor being called 7 times after "Filling list"?

B.T.W. thanks for the suggestion about changing the name of list. Good point.

[Paul McKenzie]

I'm a little confused at this point. . . I've rewritten my code (before having a chance to read your response). It seems like vector is actually deleting the objects. . . Here is the code:Did you read my post completely? I will repeat it again:
Fifth, a std::vector is non-POD (Plain-Old-Data). You cannot use functions such as iostream::write() to output the vector's contents, or read() to read in values. This is C++, not 'C'. To input/output values of non-POD classes, you must do something called serialization. The easiest form of this is to write out or read in the values one-by-one in a loop.You are still using read() and write() to output the raw vector object. This is incorrect. Again, vector is a non-POD C++ class, it is not a C struct.


in.read((char*) &list, sizeof(class vector<MyClass>));
cout << "Load complete. yippee." << endl;
There is no "yippee" here. You are invoking undefined behavior, and lucky your code doesn't crash. Get rid of that code before you test again, and replace it with reading the values, one at a time, and storing them in the vector by calling push_back(). Similarly, get rid of that code that outputs to the file, and replace with the version I posted.

1) You can't just overlay a vector object with raw data by using fread(), memcpy(), etc. You are corrupting the object if you did this. I need to say this again, since you didn't understand the first time. Only use the functions that are provided by std::vector to add elements to it (i.e. push_back(), insert(), resize(), etc.)

2) The sizeof(std::vector<MyClass>) doesn't change. You can have no MyClass objects in the vector, or a million of them. The sizeof() will not change, since sizeof() is a compile-time constant. Think about this, and you see why your code is invalid.

You are also not calling "delete" on the memory you allocated for your objects. Therefore you have a memory leak (I mentioned this in my first post also).

. . . and here are the results:

Those results are invalid until you write a valid, well-formed C++ program that does not introduce undefined behavior.

Regards,

Paul McKenzie

[paradoxresolved]

Paul,

Thank you again. I DID read your post the first time. I proceded to post my altered code because I want to understand why it is behaving as it is, with regard to the current (albeit flawed) code. I understand that you cannot write() to file a vector in that way, but what I don't understand is why the destructor of MyClass is being called even with the memory leak still there.

I'm not saying I don't believe what you've said. I just don't understand why the destructors are still being called. Specifically:

1) Why are the destructors being called 7 times at the start?
2) Why are the destructors called 5 times after clear()?
3) Why are they called 5 times at the end?

I've always been under the impression that you could tell if a dynamically allocated object has been deleted by watching for a call to its destructor. Am I incorrect in this?

Thanks again, and please be patient. I'm trying to learn this.

p.s. the "yippee" was not an exclamation of joy . . . but an outcry of exhaustion and confusion.

[paradoxresolved]

. . . also, I'm not getting any kind of message about a memory leak by the compiler. I'm using VC++ 6.0, and it usually shows up under the debug tab at the bottom when the program exits. This is very confusing.

Any help in understanding this would be greatly appreciated. Thanks.

[paradoxresolved]

Does vector take the object passed to it by push_back() and copy it into dynamic memory, but NOT the object itself? I could see how this might eventually call the destructors for the copied entity while leaving the original object as a memory leak. . . two "new"s and only one "delete". But why no memory leak message from the compiler?

Thoughts?

[Paul McKenzie]

Paul,

Thank you again. I DID read your post the first time. I proceded to post my altered code because I want to understand why it is behaving as it is,
Once you mess up the vector by overwriting it with the read(), then all of your results are invalid. Again, the behavior of your program becomes undefined once you do this. So it makes no sense to try to figure out something that is corrupted. What good does that help you?
but what I don't understand is why the destructor of MyClass is being called even with the memory leak still there.You are storing MyClass objects in a vector. A vector makes copies of the object you store in it, therefore the construction and destruction. The vector is destroying its own copies.

1) Why are the destructors being called 7 times at the start?
2) Why are the destructors called 5 times after clear()?
3) Why are they called 5 times at the end?
Anything after the read() occurs, you can throw away due to corruption of the vector.

Regards,

Paul McKenzie

[Paul McKenzie]

. . . also, I'm not getting any kind of message about a memory leak by the compiler. Compilers don't give messages about memory leaks. They tell you if the syntax of your code is correct.

You don't need any tools to see there is a memory leak. You called "new", but you didn't call "delete" -- memory leak.

Regards,

Paul McKenzie

[Paul McKenzie]

Does vector take the object passed to it by push_back() and copy it into dynamic memory, but NOT the object itself?That's correct. Think for a moment -- can you write code that creates an object, and then move the *very same object* to another area of memory? Of course not -- you must *copy* it. There is your answer -- the vector must make a copy of the object.
But why no memory leak message from the compiler?Again, compiler's do not give error messages about memory leaks. You are talking about some leak detection program that is independent of the compiler. Anyway, I'll say it again -- you don't need tools for this. The code is right there for anyone to see that you have a memory leak, regardlesss of what a "memory leak reporting tool" says.
Thoughts?Determine memory leaks by looking at your code, especially the obvious ones. Memory leak tools are used if the program is complex and memory leaks are not easily detected by doing a sight examination of the code. They should not be used to tell you if a trivial program is correct or not.

Regards,

Paul McKenzie

[HighCommander4]

Just to expand on what Paul McKenzie said about read() and write() not working with vectors:

When you have a structure like this:

struct mystruct
{
int member1;
int member2;
};

This structure simply takes up 8 bytes of contiguous memory (assuming a 4-byte int and no structure alignment beyond 4 bytes). When you use write() to write the contents of this structure to a file, you are asking the computer to interpret the 8 bytes taken up by this structure as 8 characters, and writing the 8 characters to the file. Then, when you read() this structure from the file, what you are really doing is reading 8 characters, and interpreting those 8 bytes taken up by the 8 characters as a 'mystruct' structure. This works fine as long as the read() is done on the same system as the write() was done (whereas, if the write() was done on a little-endian system and then read() is done on a big-endian system, the integer values read from the file would be different from those written).

So simple structures like this can be safely written/read from files using read() and write(). (Note that structure alignment does not prevent this from working as long as you use sizeof(mystruct) to determine how many bytes to write, because sizeof() takes into account the structure's alignment).

One important thing to note is that the size of this structure is known at compile time. One of the rules of C++ is that the size of any structure in memory (as returned by sizeof()) must be known at compile time.

The problem happens when you have structures that use dynamic memory. Let's use a simplified version of vector as an example:

template <typename T>
struct vector
{
int size; // how many elements the vector has
T* data; // points to the dynamic memory used to store the elements
... // member functions, etc.
};

We know that a vector can store an arbitrary number of elements. The way vector accomplishes this is by dynamically allocating the memory used to store its data, and maintaining a pointer to this dynamic memory. This way, the vector structure itself consists of only an integer keeping track of how many elements the vector has, and a pointer to the dynamic memory used to store these elements. Assuming a 4-byte int and a 4-byte pointer, this yieds a constant structure size of 8 bytes (remember, one of the rules of C++ is that the size of a structure must be known a compile time and thereofre must be constant). [In reality, the size of the vector structure may be more than 8 bytes - the code above is just for the sake of the example]. It doesn't matter if the vector holds 0 elements, 1 element or 1000 elements - sizeof(vector) will still be the same.

Now, here is the problem with using write() and read() with vector. When you do something like this:

ofstream fout("some_file");
vector<int> vec;
... // populate vector
vec.write((char*)&vec, sizeof(vec));

the only thing being written to the file is that 8 bytes storing the number of elements and the address where the elements are stored in memory. The actual elements themselves are not written to the file. When 'vec' is destroyed (at the end of the program, for instance), the memory storing the elements is lost. Then, when you read() the vector back into memory using the same technique, it will read the 8 bytes and interpret it as the number of elements and the pointer to the elements in memory - except that pointer points to memory that is long gone and is probably already being used for something else.

The correct way to write a vector to a binary file, then, is to iterate through the vector and write each of its elements individually:

ofstream fout("some_file");
vector<int> vec;
... // populate vector
for (int i = 0; i < vec.size(); ++i)
fout.write((char*)&vec[i], sizeof(vec[i]));

And the corresponding way to read the elements into a vector:


ifstream fin("some_file");
vector<int> vec;
int temp;
for (int i = 0; i < number_of_elements; ++i)
{
fin.read((char*)temp, sizeof(temp));
vec.push_back(temp);
}

Note that for reading the vector, you need to know how many elements are to be read, so you may want to, before writing the contents of the vector to the file, write an integer value keeping track of how many elements are being written to the file.

[RoboTact]

Again, compiler's do not give error messages about memory leaks. You are talking about some leak detection program that is independent of the compiler. Anyway, I'll say it again -- you don't need tools for this. Just to avoid confusion - VC++ debugger does have memory leak detection mechnism - but again, this mechanism works only if data isn't corrupred - it uses memory around allocated data block to store information required to detect memory leaks. Once you corrupted the data, you can't get correct leak detection.

When you write push_back(*(new MyItem())), you allocate memory, then copy its contents to vector. On clear() copy's destructor is called, while original object allocated in heap isn't destructed.

[paradoxresolved]

Wow! Thanks for the help! It's making much more sense now. The actual program I'm working on contains lists of objects, each of which contain lists of objects, and so forth. . . sometimes 4 layers deep. Each object contains pointers to objects in other lists, making the whole thing a virtual spider web and nightmare to save/load to file. Until now I had been working with my own List<> template and saving it via text to a .txt document and using an ugly looking codex to get the information out. I had been avoiding vector<> because I didn't know how it worked on the inside, and don't like black boxes (when I have the choice to avoid them that is). Anyway, I've been moving towards making the code standard and straight forward to others and have been thinking about switching to unformatted saving . . . you guys answered my questions really well, and I think I'm atleast on the right path to that solution. Thanks again.

[Marc G]

Sorry Calculator. . . I was trying to figure out how to format my posting. I did not mean for that to be posted as it was. If a moderator is reading, please change the title to "new to vectors, problems". Thank you, and I apologize for this trouble. I'm being rushed on someone elses computer. :(
I have renamed the thread ;)

[Paul McKenzie]

I had been avoiding vector<> because I didn't know how it worked on the inside, and don't like black boxes (when I have the choice to avoid them that is).I've heard this argument before (usually from C programmers), but it is easily invalidated.

You use fopen(), fclose(), along with passing this FILE thing to various functions, right? Isn't that a "black box" in that you don't know what's going on inside?

How about qsort()? What about sqrt()? Do you know what algorithms are used to compute sin(), log()? Are these "black boxes"? How about a simple strcpy()? How about print(), scanf()? More black boxes.

What about directory traversing functions? In Windows you have GetFirstFile and GetNextFile. More black boxes.

In 'C', you've been using black boxes all the time. Using a vector, IMO is no different than using FILE, fopen(), printf(), or any of the other structures and functions that you normally use in programming.

Some group of people coded the std::vector, some group of people also coded fopen(). I don't know why so many (again usually 'C' programmers going to C++) put the trust that fopen() or printf() works without any hint of doubt, but a vector or list brings all sorts of doubt (even if the same people coded the entire library, including the C and C++ libraries).

Regards,

Paul McKenzie