I'm fairly new to C++, so the code should look very simple to the guru crowd. When I try to compile the short program below, I get the error that "struct std:: ofstream has no member function 'getline'". I've checked around a bit and this line is used fine elsewhere, as well as in the book I'm learning from. Here's the code, with the problem area bolded:

I suggest you use std::getline instead of ifstream.getline and use std::string instead of char.

02-02-2008

PAragonxd

Heh, I knew it would be a small stupid error like that. Thanks.

02-02-2008

Elysia

I also see you're using eof() as condition in the header for the loop. That's also bad. For details, you can see the FAQ. It's better to check if the read succeeds or not.

02-02-2008

PAragonxd

Thanks Elysia. As I was reading over the code a last time before posting, I thought of that line in your sig when I went over the eof() part, heh.

02-02-2008

PAragonxd

The original problem referred to by this thread was fixed, and since then a new problem arose.

Going along with my book, one of the exercises is to make the code that I posted in the first post print the designated file in all lowercase letters (the file reads AAAA\n BBBB\n CCCC\n etc). The problem code that I have now compiles fine, but when run, it ignores the requested number of lines to be printed and instead prints one line every time (the change from capital to lowercase works fine). Here's the code:

if (file_in.eof()) {
cout << "The end of the file has been reached.";
break;
}
}

cin.get();
return 0;
}

Thanks in advance.

02-02-2008

anon

Code:

for (i = 0; i < lines; i++) {
file_in.getline(input_line, 100);

for (i = 0; i < (strlen(input_line)); i++) {

Wonder what messes up loop counter?

02-02-2008

PAragonxd

Slap me hard. Please.

Thanks lol..

02-02-2008

anon

May I also point out that strlen is a bit expensive function to call a lot (it has to loop over the entire array to determine the length). In fact you needn't find out the length at all and simply loop converting characters to lowercase until you get to the null-terminator of input-line.

02-02-2008

Elysia

Dare I say it will be optimized by the compiler in release mode?
Then again, maybe not. I suppose the compiler cannot assume that strlen returns the same value each time.
But anyway, unless time is critical, it's not really necessary. So we could just say it's good practice.

02-02-2008

PAragonxd

Just for reference, how would I go about making it so strlen is only called once (seeing as the line of text that strlen is measuring is called within the 'for' loop) ?

02-02-2008

Elysia

Store it in a variable and use the variable in the loop condition.

02-02-2008

cyberfish

Quote:

Dare I say it will be optimized by the compiler in release mode?

I just tried it with gcc 4.1 some time ago.

It only gets optimized on -O3, not even -O2.

02-03-2008

anon

To convert every character in a C-string to lowercase, the loop might look like this:

Code:

for (int i = 0; str[i] != '\0'; ++i) {
str[i] = tolower(str[i]);
}

Of course, you might use pointer arithmetics instead of indexed access. However there is no need to call strlen for that at all. Both strlen and this loop work the same way: keep looping until the current character is '\0'.

-----------
And one more note on coding style. It might be wiser in C++ to declare variables at the smallest scope you are using them. In case of for-loops the loop counter can be declared as part of the for statement (see example above).

As to your mistake of using the same variable i for two nested loops - had you declared the two i's in the for statements a) the compiler could have warned you about problems with variable names, and b) if you ignored the warnings the code would have still done the right thing. (A variable declared in a nested scope shadows the variable of the same name in the outer scope.)