Identical file loading routine loads the files differently

Started by
8 comments, last by Zahlman 18 years, 10 months ago
I dont know what Ive done but, I got two files with a couple of lines in them (names and phrases for npc merchants) both needs to be loaded into the program. One is loaded fine but the other one isnt :S. I cant see no difference in the way I load them. Heres the npc database constructor:

NpcTraders::NpcTraders() {
	this->numberOfPhrases = 0;
	this->numberOfTraders = 0;	// just in case

	char temp[255];		// temp space
	std::ifstream file;	// the file handler

	// First the names
	file.open("Names.txt");

	if(file.bad() == true) {			// if file didnt load, abort
		cout << "Unable to load file Names.txt";
		return;
	}

	// how many should we read?
    file >> this->numberOfTraders;
	cout << this->numberOfTraders;
	for(int i = 0; i < numberOfTraders + 1; i++) {	// run until end is reached, +1 since we pop the first one later					
		file.getline(temp, 30, '\n');				// place in temp space	
		name.push_back(string(temp));				// then add them at the end of the vector
		if(file.eof())								// if file is at the end, break
			break;
	}
	name.erase(name.begin());					// since the first one is empty we delete it
	file.close();								// close the file

	// -------------------------------------------------------------------------------

	// Then the phrases
	file.open("Phrases.txt");

	if(file.bad() == true) {			// if file didnt load, abort
		cout << "Unable to load file Phrases.txt";
		return;
	}
	
	// same as above
	file >> this->numberOfPhrases;
	cout << this->numberOfPhrases;	
	for(int i = 0; i < numberOfPhrases + 1; i++) {		// run until end is reached							
		file.getline(temp, 255, '\n');		
		phrase.push_back(string(temp));
		if(file.eof())
			break;
	}
	phrase.erase(phrase.begin());
	file.close();

	this->debugOutput();
}


The "Names.txt"

8
Greger
Jossan
Aboo
HansMamma
Rupert
KarlEvert
Nisse
Hampus


The number at the top tells how many entries there is in the file and thus letting the program know how many it needs to load. The "Phrases.txt", the ones that not loaded

5
OMG!!
lol... 
thats just... wierd... dude
hello world
i really dont have any fantasy!
your mom is a dad... >:)


Any ideas?
Advertisement
Just wondering. Did you intentinoaly(sp?) put 5 in Phrases.txt when you have 6 lines bellow? If not that might be your problem. (Just a guess sence I didn't read your code indeapth.)
Have you tried stepping through with the debugger?

Also, You have 8 names in he first file and 6 phrases in the second, although you indicate 5 in the "header."
It was a typo... but sadly it wasnt the solution. If I put in a cout statement to get the value it just says zero :S like it couldnt read the file (ive set them zero in the beginning of the function you see). But shouldnt that trigger the error checkig if there?
Reopening a stream doesn't clear the EOF flag. It is an acknowledged library defect. Do a file.clear(); before the second file.open();

Some issues as I read your code:
* Use initializer lists instead of setting members in a constructor body.
* Use std::getline instead of std::istream::getline.
* Constructor failure should be done with exceptions, not returns.
* Error messages are sent to cerr, not cout.
* Remember that a std::deque has a pop_front - in fact you might even want to just skip the first element instead of discarding it later.

A better design would have a separate loading function that gets called twice by the constructor.
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan
Thanks Fruny that solved it. But I dont know what a initializer list is... i dont know about most of the stuff you wrote there. :P Im learning as I go, please bear with me ;). Im satisfied if it just works, I dont want to add to much foreign stuff. Easier to learn if its... uhm... easier.
Quote:Original post by Mizipzor
But I dont know what a initializer list is...


class Foo{   int i,j,k;public:   Foo(int w) :       i(100),       j(42),       k(w*100)   {   }};


The difference between setting the member variables in an initalizer list and setting them in the constructor's body is that with the initializer list, they are set when the object is constructed. Setting them in the class body means that first the members get default constructed (e.g. set to zero for int members), and then overwritten with the new value. For basic types it doesn't matter much, but this can quickly become very expensive. Furthermore some types cannot be assigned to (e.g. a reference or a const member), only initialized - or cannot be default constructed (e.g. class without default constructor, like Foo above) - which means you must pass the proper parameters to the member's constructor to do the initialization. This is done with an initializer list.

Quote: i dont know about most of the stuff you wrote there. :P Im learning as I go, please bear with me ;).


Hey, I just gave you an excellent opportunity for you to at least look up what I talked about. I'd be rather disappointed (i.e. "Am I wasting my time here?") if you didn't.

Quote:Im satisfied if it just works, I dont want to add to much foreign stuff.


Quick fixes have a tendency to come back and bite you in the arse later. Better to learn how to do things correctly from the start.

Quote:Easier to learn if its... uhm... easier.


Yeah, but the Dark Side isn't more powerful, just more seductive. If you want to make programming your profession, you'd better make sure you learn do to things the Right Way™ rather than the easy way.
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan
Quote:Original post by Mizipzor
But I dont know what a initializer list is... i dont know about most of the stuff you wrote there.


A initializer list are the correct method of initializing data members of class/struct in its constructor e.g.

struct foo {   int bar, foobar;   foo(int a = 0, int b = 0)   : bar(b), foobar(b) {}};


Quote:Original post by Mizipzor
:P Im learning as I go, please bear with me ;). Im satisfied if it just works, I dont want to add to much foreign stuff. Easier to learn if its... uhm... easier.


I just like to point that there is a getline for std::string its a free function and not a member function of istream. Also i noticed that you are erasing the first element of your vectors/deques but i can't see why you need to.

I had written an alternative to your code for you but now i'm not sure if should post it [grin], anyways if your interested:

#include <algorithm>	// copy, for_each#include <iterator>	// istream_iterator#include <vector>	// vector#include <string>	// basic_string#include <fstream>	// basic_ifstreamstruct string_reader {	   const char delimiter;   std::istream& in;   string_reader(std::istream& in_, const char d = '\n')   : in(in_), delimiter(d) {}   std::istream& operator()(std::string& s) const {	return std::getline(in, s, delimiter);   }};int main() {   using namespace std;   typedef vector<std::string> vec_of_strs;   typedef istream_iterator<std::string> isstr_itr;   vec_of_strs names;   vec_of_strs::size_type N = 0;   ifstream in("temp.txt");   in >> N;   names.reserve(N);   copy(isstr_itr(in), isstr_itr(), back_inserter(names));   in.close();   in.clear();   in.open("temp2.txt");   in >> N;      vec_of_strs phrase(N);   std::for_each(phrase.begin(), phrase.end(), string_reader(in));}
Fruny, ah man you know I listen very carefully to what you say! You've helped me out before ;) Ive rewritten my code now. Thanks for the tips and explanations.

And thanks to you to, snk_kid :) another view or explanation is always helpful. :)

/me bows.
Note that snk_kid's example demonstrates two different techniques for reading the file data - using one for each file. (Actually, two separate techniques for sizing the vector, as well; and you may mix and match.) You should prefer to pick one of them, make a function for it and then call it twice, in order to avoid duplication of code. (It will also avoid the need to .clear() the fstream, since there will be a separate fstream for each function call.)

As for the reading n+1 lines and then removing the first - it really would be a lot easier to just skip past the newline (the one right after the file length count) before starting to read the strings. You could do a dummy getline() call, make use of istream.ignore() (you know, that thingy you use when trying to prompt for a number on cin :) ), or in this case just "file >> ws" - ws stands for whitespace, and it's a stream manipulator that will basically just "eat" all whitespace at the current point in the file.

Also consider that by making use of a vector (or other library container) you may not need to store a length count in the file at all - nor in your object (you could just use the container's .size() ).

This topic is closed to new replies.

Advertisement