[C++] Problem with ignoring input filestream

[Lord Fear]

As the title says, I have a problem with my programming assignment. It for some reason stops reading from the file.

bool PlayerRegister::loadFromFile(string fileName)
{
	ifstream fin = ifstream(fileName);
	if(fin)
	{
		this->clearAllocation();
		fin >> this->capacity;
		fin >> this->nrOfPlayers;
                // Those two works just fine.
		this->players = new Player*[this->capacity];

		for(int i = 0; i < this->capacity; i++)
		{
			if(i < this->nrOfPlayers)
			{
				string name = "";
				int birthYear = 0;
				string phoneNumber = "";
				int shirtNumber = 0;
				string playerPosition = "";

				getline(fin, name);
				fin >> birthYear;
				getline(fin, phoneNumber);
				fin >> shirtNumber;
				getline(fin, playerPosition);

				// The above lines don't do shit. If I give all the variables random default values none but name gets changed. And it becomes empty. 

				this->players[i] = new Player(name, birthYear, phoneNumber, shirtNumber, playerPosition);
			}
			else
			{
				this->players[i] = NULL;
			}
		}
		fin.close();
		return true;
	}
	else
	{
		fin.close();
		return false;
	}
}

Since I create the files myself I know that they are structured this way.

Does anyone have an idea why this is happening? If I set a break-point at all of the lines it gets triggered, but it never reads anything from the file.
I've tried checking if the next char is a \n before it reads but that didn't help.

[ZeekyHBomb]

Can you post a file? Then I'd just try it by myself :)

A few other notes:
You don't need to close the file by yourself, the destructor will do that.
Take the string by reference, this way it doesn't need to get copied.
Create the variables outside the loop. This way, they don't need to get reinitialized at each iteration.
A std::vector is a neat C++-style dynamic array. If you still prefer your Player**, you could still just make a simple Player* and use placement-new, e.g.
[cpp]this->players = new Player[this->capacity];
//...
new (this->players + i) Player(...); //this will create the Player object at this->players[i][/cpp]

[Lord Fear]

10
2
First Name
1991
1234567
2
Forward
Second Name
1994
1234567
3
Center

That's the file I'm reading.

And I had the variables outside the loop at the start, I moved them in to see if it made a difference.
Same with the close(), I didn't have it at first but I put it there to see if it made a difference.

And the reason I'm using a Player** is because it's in the specifications of the assignment.

[ZeekyHBomb]

Also, it will be more efficient to iterate over the elements via pointer, instead of using the index.
[cpp]Player *const *const playerEnd = this->players + this->nrOfPlayers;
for(Player *cur = this->players; cur != playerEnd; ++cur)
{
//...
new (cur) Player(...);
}

std::memset(playerEnd, NULL, this->capacity - this->numOfPlayers);[/cpp]
And directly memset the NULL-values instead of iterating over all of them :)

[editline]23rd November 2010[/editline]

Okay, I'll check it out now...

[Lord Fear]

[QUOTE=ZeekyHBomb;26246810]Also, it will be more efficient to iterate over the elements via pointer, instead of using the index.

And directly memset the NULL-values instead of iterating over all of them :)
[/QUOTE]

Unfortunately, I have to do it the way my teacher has showed us.

[ZeekyHBomb]

Your checking for a new-line should actually have revealed the issue.
After each reading of input via >> a new-line character is left. The getline-function discards of it though.
The reason the variables stayed the same is because name was \n and birthYear was tried to being read as "First", thus the failflag was set and subsequent calls to operator>> or getline did nothing.

And surely you could still present your better code to the teacher? If you have questions about my suggestions, I'd be glad to elaborate.
If you have a close-minded teacher though .. my sympathy.

[Lord Fear]

[QUOTE=ZeekyHBomb;26247127]Your checking for a new-line should actually have revealed the issue.
After each reading of input via >> a new-line character is left. The getline-function discards of it though.
The reason the variables stayed the same is because name was \n and birthYear was tried to being read as "First", thus the failflag was set and subsequent calls to operator>> or getline did nothing.
[/QUOTE]

Hmm... I wonder what the fuck I did the first time when checking. Because it worked this time.
Thanks :)

[QUOTE=ZeekyHBomb;26247127]
And surely you could still present your better code to the teacher? If you have questions about my suggestions, I'd be glad to elaborate.
If you have a close-minded teacher though .. my sympathy.[/QUOTE]

Well, he isn't really close minded. But the assignments are pretty specific on how we're supposed to do stuff.

And it's basically to show that we've learned what he's told us. Even if I know a better way I need to show I know his way too.

[Wyzard]

[QUOTE=ZeekyHBomb;26246706]If you still prefer your Player**, you could still just make a simple Player* and use placement-new[/QUOTE]
I wouldn't recommend that. Creating the array of players automatically default-constructs each one, so the call to placement new is overwriting fully-constructed objects without destructing them first. Just use the objects that were default-constructed by the new[] call; there's no need to reconstruct them.

Placement new is also likely to be confusing to beginners because it breaks all the rules about destruction: you [i]don't[/i] use the delete operator on something constructed with placement new, you directly call the object's destructor instead. (In this case you wouldn't do that, you'd just call delete[] on the array, but that's only because of the strange and unnecessary way that placement new was used to begin with.)

[Lord Fear]

We actually did learn a dynamic array at first
Player *players = new Player[this->capacity]
But having an array of pointers to objects will optimize it since you never actually create a new object unless it's used. In the normal dynamic array it creates empty objects of Player that just takes up more ram then needed.
It also makes the operation of extending the array more resource heavy since you have to copy over all the elements in a normal dynamic array. While in a pointer array you only have to switch over the pointers and never touch the actual object.