TXT File Loading Prog

[Gromnie]

my 2nd prog, i intend on adding to it next so that it manipulates the data, see what cool things i can make it do....anyways, just looking for some feedback again on my prog, does it suck :) ? Thx in advance

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

using namespace std;
class WordRead
{
public:
	WordRead();
	~WordRead();
	void EnterName();
	void LoadFile();


private:
	char ch;
	char theFile[1000];
	ifstream myFile;
	vector<char> load;
};

WordRead::WordRead()
{
}

WordRead::~WordRead()
{
}

void WordRead::EnterName()
{
	cout << "Type in the text file path you wish to read: ";
	cin >> theFile;
}

void WordRead::LoadFile()
{
	myFile.open(theFile);
	for(int i = 0; !myFile.eof(); i++)
	{
		myFile.get(ch);
		load.push_back(ch);
		cout << endl << "char " << i << ": " << load[i];
	}
	myFile.close();

}

[Duoas]

The only two things I see that are guaranteed problems are first in EnterName(). You should input text using getline().

void WordRead::EnterName()
{
	cout << "Type in the text file path you wish to read: ";
	cin.getline( theFile, 1000 );
}

And second in LoadFile(), your loop will never terminate if you never hit EOF (which can happen if a read error occurs). A better idiom is to terminate if the istream indicates any failure.

for(int i = 0; myFile.good(); i++)

But you've also got another problem. Even if you hit EOF, you still try to add a character to the end of your vector. As an added note, using the class variable ch is much slower than using a local char variable. Below I suggest something better for you.

You should really consider making use of the STL string library. That will make things much easier, and you'll get better performance than a vector of char.

I think you need to work on your variable names also... Variables should be nouns, and functions should be verbs. Both should be descriptive of what it represents.

...
#include <string>

...

private:
	// This is more descriptive: it tells you it is the
	// file's -name-, and not some other thing.
	string theFileName;
	// Fine
	ifstream myFile;
	// Again, this tells you that it is the file's text.
	// 'load' is a verb, and is doubly confusing when
	// used anywhere other than in LoadFile().
	// (It is confusing there too, though..)
	string theFilesText;
...

void WordRead::EnterName()
{
	cout << "Type in the text file path you wish to read: ";
	getline( cin, theFileName );
}

void WordRead::LoadFile()
{
	char c;
	myFile.open(theFileName);
	for (streamsize i = 0; myFile.get( c ); i++)
	{
		theFilesText.push_back( c );
		cout << "char " << i << << ": " << c << '\n';
	}
	myFile.close();
}

I use '\n' instead of endl because the second forces a flush every time, while the first only flushes when cout thinks it is a good time. This will make your file load much faster also.

Whew. Nice use of classes, and you seem to have a good idea of structure. Hope this helps.

[Gromnie]

I made the changes suggested but ran into a couple problems. This is the prototype of the fstream.open function.

void open(const char *filename, openmode mode = in | out);

so it will not recieve it as a string unless i change it to....

myFile.open(theFileName.c_str());

if i understand this right, that just changes it back into chars? would it be worth it to use that even or just have it as a char in the first place....or is there something i'm missing, another way to use it as a string?

then we have this...

string theFilesText;
char c;
for (streamsize i = 0; myFile.get( c ); i++)	
{		
theFilesText.push_back( c );		
cout << "char " << i << << ": " << c << '\n';	
}

that also has issues with switching from a string to a char...haven't figured out how to fix it yet, i'd assume it would have something to do with using a template, haven't messed with those really yet.

[Duoas]

That's not a problem. Underneath a std::string or a std::vector is just an array anyway, except that all the nasty details of managing it are taken care of for you.

So, yes, use c_str() to get a const char* back that you can use with functions that take them. If your STL is implemented "properly" (IMO) then it shouldn't cost you anything to do it that way.

It looks like the second problem is that I accidentally typed << twice in a row. You can't have two binary operators in a row without an interleaving argument. It has nothing to do with strings. So, change line 32 to:

cout << "char " << i << ": " << c << '\n';

Sorry about that! :$