With the following line I get a syntex error relating to the ifstream file

if ifstream file(filename.c_str()).find string1
	   ++counter;

I have already declared the input file and do not understand why there is an error. Sorry for seeming dumb but I am new to this!!

I'm trying to see if a pre-declared string is in the input file and if it is increment the counter

Many ways to do it, here's one. (Off the top of my head). Even if your syntax were correct, you actually didn't read any file contents before comparing it's content...

[/left]
[left]string string1="WHATEVER";[/left]
ifstream file(filename.c_str());
 
if ( !file.is_open() )
		 return; // or otherwise stop execution
string buffer;
 
while ( ! file.eof() )
{
		 getline( file, buffer );
		 if ( buffer.find( string1 ) != -1)
				++counter;
}
file.close();

>> while ( ! file.eof() )
You shouldn't use <stream>.eof() as a loop condition. Because the eofbit is only set after a request for input has failed, you'll increment the counter once more than necessary if the last line contains the search string. A better way to read input is to use the return value of your input function:

while (getline(file, buffer)) {
  if (buffer.find(string1) != string::npos)
    ++counter;
}

>> if ( buffer.find( string1 ) != -1)
Okay, this is a subtle error. string::npos is defined as (size_t)-1, which is not the same as -1. Your test may or may not work as written, so you should use string::npos directly for tests such as this one.

>> file.close();
There's no need to close an fstream explicitly if the destructor is going to be called. Just FYI.

I'll agree with the first 2 corrections . With the 3rd, I prefer to close explicitly and not wait, to better avoid possibly hitting the maximum open files of the operating system. (In case I have a program that opens many files at a time)

I agree, but only if the fstream object doesn't go out of scope before opening another. There's no need to close the stream if this is your code:

int main(int argc, char **argv)
{
  for (int i = 1; i < argc; i++) {
    fstream file(argv[i]);

    if (!file) continue;

    // Process the file
  }
}

There's no way that you can have more than one stream open at the same time because the destructor is called after each iteration of the loop. The destructor will flush unwritten output and close the stream properly. The only reason you would have for calling close explicitly is if you reuse the fstream object for another file, have enough fstream objects to reach an implementation limit of open files, or the fstream object will be unused, but still in scope for a while after processing the file.

But if you like the extra typing and redundant code, who am I to stop you? Some people prefer to close their file streams explicitly as a matter of style.

But if you like the extra typing and redundant code, who am I to stop you? Some people prefer to close their file streams explicitly as a matter of style.

I guess that's me! My thought is that if they didn't want me to close it, they would have made the close function private or protected.

>> My thought is that if they didn't want me to close it, they would have made the close function private or protected.
So you always use the open function too even though the constructor provides that functionality?

std::fstream file;

file.open(filename);

// ...

file.close();

If they didn't want you to open the file then they wouldn't have made it public. Since you're adding redundant code for no reason, you might as well make it explicitly symmetrical. ;)

Yes, I'm mocking you. No, it's not personal. No, it's not a flame or an insult, so don't take it as such.

Not insulted, enjoying your banter. :surprised

(By the way, I've got some other posts that could badly use your help in answering, feel free to expend your energy on helping me there...)

This question has already been answered. Start a new discussion instead.