Filling a vector with several objects of the same type

[DMDTT]

Hello guys, i've just registered and i hope to find a fast solution.

my job is to read in a master data file of several persons.
this data should be display by a winform gui...


this is the structure of the file is
id;surname;forename;size;sex
1;mustermann;max;1.95;m

now my problem:

// PersonalVerwaltung.cpp: Hauptprojektdatei.

#include "stdafx.h"
#include "Form1.h"
#include "Person.h"
#include <vector>

using namespace PersonalVerwaltung;


void readPersonsFromFile(vector<CPerson>*);


[STAThreadAttribute]
int main(array<System::String ^> ^args)
{
	// Aktivieren visueller Effekte von Windows XP, bevor Steuerelemente erstellt werden
	Application::EnableVisualStyles();
	Application::SetCompatibleTextRenderingDefault(false); 

	//personList = 
	vector<CPerson>* personList= new vector<CPerson>;
	readPersonsFromFile(personList);
	// Hauptfenster erstellen und ausführen
	Application::Run(gcnew Form1(personList));

	
	return 0;
}

void readPersonsFromFile(vector<CPerson> *t_PersonList)
{
	/*	Funktioniert mit direkter Zuweisung
	CPerson *t_Person = new CPerson(1, "Mustermann", "Max",1.95, "m");
	t_PersonList->push_back(*t_Person);
	t_Person = new CPerson(1, "Musterfrau", "Birgit",1.73, "w");
	t_PersonList->push_back(*t_Person);*/
		
	/**Variante mit auslesen aus Datei****************************************/
	int i = 0;			//Laufindex für Auslesen
	char buffer[150];
	FILE *f;
	
	f=fopen("Personal.txt","r");
	for(i=0;fgets(buffer,sizeof buffer,f)!=NULL;i++)
	{
		t_PersonList->push_back(*new CPerson(atoi(strtok(buffer,";")),
											strtok(NULL,";"),
											strtok(NULL,";"),
											atof(strtok(NULL,";")),
											strtok(NULL,";")));
		
	}
	
}

if i code it like this, the vector is full of one data -->only one person in every row...in debuggin mode, i see, that in every row the addresses for every property is the same...
i've tried to avoid this by taking

t_PersonList->push_back(*new CPerson(atoi(strtok(buffer,";")), ...etc.

any suggestions?

and the second problem is:
in the main method i'm creating a vector personList--> this is given to the reader-method by "call by reference"

nethertheless the temporary t_personList is filled all with the same data...
the "main" personList has the same contents except the char-fieds...
in debugging mode there is ""

my latin is over...

hope you can understand me :-)


aah i forgot...i'm using visual studio 2010

[Abramus]

Your program has a few problems.

First of all, order of evaluation of a function's arguments is undefined. But calls to strtok need to be performed in a very specific order. So you need to move these calls outside of CPerson constructor. Additionally, using new to allocate CPerson will cause memory leak. Modify the code as follows:

   for(i=0;fgets(buffer,sizeof buffer,f)!=NULL;i++)
   {
      int          id       = atoi(strtok(buffer,";"));
      const char * surname  = strtok(NULL,";");
      const char * forename = strtok(NULL,";");
      float        size     = atof(strtok(NULL,";"));
      const char * sex      = strtok(NULL,";");

      t_PersonList->push_back(CPerson(id, surname, forename, size, sex));
   }

Other things: I cannot see implementation of CPerson::CPerson, but I assume that it copies strings, that are pointed to by its arguments, into local buffers (DOES NOT copy only pointer values). Additionally, you should be aware that for the file format you specified, "sex" strings will contain newline characters (i.e. "m\n" instead of "m").

Hope that helps.

[DMDTT]

thx for your answer...
I'm quite a newbie...why does it cause a memory leak??... i'm coming from java, you know^^

[DMDTT]

this is my person-class

#include "StdAfx.h"
#include "Person.h"


CPerson::CPerson(void)
{
}

CPerson::CPerson(int t_Id, const char *t_Surname, const char *t_Forename, float t_Size, const char *t_Sex)
{
	iId = t_Id;
	cSurname = t_Surname;
	cForename = t_Forename;
	fSize = t_Size;
	cSex = t_Sex;
}

void CPerson::setId(int t_Id)
	{
		iId = t_Id;
	}
int CPerson::getId()
	{
		return iId;
	}

void CPerson::setSurname(const char *t_Surname)
	{
		cSurname = t_Surname;
	}
const char* CPerson::getSurname()
	{
		return cSurname;
	}

void CPerson::setForename(const char *t_Forename)
	{
		cForename = t_Forename;
	}
const char* CPerson::getForename()
	{
		return cForename;
	}

void CPerson::setSize(float t_Size)
	{
		fSize = t_Size;
	}
float CPerson::getSize()
	{
		return fSize;
	}

void CPerson::setSex(const char *t_Sex)
	{
		cSex = t_Sex;
	}
const char* CPerson::getSex()
	{
		return cSex;
	}

so i've changed to your solution...
but the problem is still there...every row in vector has the same address...an the personList has no "char" values, only ""

[Abramus]

As I said, CPerson::CPerson should copy contents of buffers referenced by t_Surname, t_Forename, and t_Sex instead of simply copying pointers. This is because the pointers point into a local buffer (char buffer[150];), whose contents is altered on the next loop iteration, and which is destroyed when readPersonsFromFile exits.

As for "new", you see in C++ there are no garbage collector. You create a CPerson object, copy it into the vector<CPerson>, but the created object is not destroyed. Moreover, its reference is lost so you are not able to destroy it. This is a memory leak - the created object will remain in the memory forever (well, until you exit the program) even through it is unused.

[DMDTT]

kk...for example?

void CPerson::setSex(const char *t_Sex)
	{
		cSex = &t_Sex;  //--> maybe like this?
	}

furthermore i don't understand, how the vector is filled...every the vector increments, but the data is overwritten with the new CPerson...

[Abramus]

OK, it looks that you don’t understand the concept of pointers.

In your current implementation, CPerson objects do not contain all these strings read from the file (surname, forename, and sex). They contain pointers to this strings. Pointers may become invalid if we destroy/overwrite data they point to. This is exactly the case here. When you read the second line of the file, contents of the "buffer" is overwritten, and thus pointers that are kept in the "previous" CPerson object (object that was added to the vector in the previous iteration of the loop) become invalid.

The solution is to store the read strings (and not pointers to these strings) in CPerson object. You can do it for example by changing types of cSurname, cForename, and cSex to std::string. Then, the line:

cSurname = t_Surname;

will actually copy the string into a CPerson object.

Alternatively, you could just get rid of all these pointers and use std::string everywhere (although then you would have to remove strtok calls too).

[DMDTT]

in conclusion...

i don't get, why i'm using no "new" by push_back CPerson...

and further...
a string is a datatype like int...or sth. ...and char is always a pointer?

[Athar]

Because you need to delete every object you created with new.

t_PersonList->push_back(*new CPerson(atoi(strtok(buffer,";"))
Here you create a CPerson object using new, which returns a pointer to this new object. You dereference the pointer, which gives you a reference to said object and you pass it to push_back. The vector stores a copy of the object.
You're not saving the pointer returned by new anywhere, so it is lost. The original object created by new still exists, without any chance to delete it.
That is called a memory leak.

and char is always a pointer?

No, char is a single character, as the name implies.

As has already been said on multiple occasions, you need to get yourself a good book about C++ (meaning the C++ Primer), before you go any further - let alone think about applying for a programming job.

[DMDTT]

thx...

you know...i'm applying for c#/ java...
and in one enterprise...a "teacher" gives the advise to develop i c++.-..so it's quite hard^^

[sadavied]

From the code you posted it looks like you are using VC++. I tend to refer to that as 'C++ .NET'.

If you want to continue using the Microsoft specific 'stuff' I would recommend using their libraries as they are managed by the garbage collector and help protect you against memory leaks etc. For instance, drop the std::vector class and use the List<> class. It is contained in System::Collections::Generic. This class has lots of useful functions for managing 'lists' of any data type.

And instead of using char* or std::string, use the managed System::String type.

For file input/output, use the classes in System::IO. It contains stream readers and writers that you can wrap around a System::IO::Filestream^ object and is MUCH easier to manage file input output.

I am not saying that STL isn't good, but if you are going to use Microsoft's tools to build GUI applications, why not use the objects they have already built for you?

[z05DSL3A]

sadavied wrote:

I am not saying that STL isn't good, but if you are going to use Microsoft's tools to build GUI applications, why not use the objects they have already built for you?

sadavied makes a good point (sort off), you should realise that if you wish to go down the route of using the .net framework you will be learning C++/CLI that is a different language to C++.

If you are intersted in the .net side of Windows programming I would suggest learning C#. If you want to learn C++ I would suggest staying away from .net (Windows Forms) and C++/CLI until you have a good grounding in C++.

[sadavied]

I agree with Grey Wolf. When I want to write GUI .NET programs I use c#. I try to stay away from Microsoft's 'version' of c++.

[DMDTT]

so and theres is the problem...

i don't know exactly which language i used...
i've done it with visual studio 2010

wether, c++/cli, visual c++, i don't know...
where can i see the diefference....

.net is quite good..huh? but i don't know how to use...

hope you understand the problem...