views:

1367

answers:

4

Do I need to manually call close() when I use a std::ifstream?

For example in the code:

std::string readContentsOfFile(std::string fileName) {

  std::ifstream file(fileName.c_str());

  if (file.good()) {
      std::stringstream buffer;
      buffer << file.rdbuf();
      file.close();

      return buffer.str();
  }
  throw std::runtime_exception("file not found");
}

Do I need to call file.close() manually? Shouldn't ifstream make use of RAII for closing files?

I've searched through most of the questions here and none were addressing this.

Thanks

+1  A: 

The best practice is to close it once you are done with it.

Shree
+20  A: 

NO

This is what RAII is for, let the destructor do its job. There is no harm in closing it manually, but it's not the C++ way, it's programming in C with classes.

If you want to close the file before the end of a function you can always use a nested scope.

In the standard (27.8.1.5 Class template basic_ifstream), ifstream is to be implemented with a basic_filebuf member holding the actual file handle. It is held as a member so that when an ifstream object destructs, it also calls the destructor on basic_filebuf. And from the standard, that destructor closes the file:

virtual ˜basic_filebuf();
Effects: Destroys an object of class basic_filebuf<charT,traits>.>  Calls close().
Eclipse
+1 - At least one who knows what RAII means :-)
milan1612
+1 I didn't know that RAII handles that...I guess you learn something new everyday
TStamper
Using a nested scope just to close the file is completely artificial - if you mean to close it, call close() on it.
anon
@Neil - good point, it's a habit I've picked up from using a lock library that doesn't have a non-destructor release method. If there's a function that lets you close it, go ahead and use it.
Eclipse
Although, you might be able to argue that restricting the object's lifetime to the necessary scope means that you won't accidentally access a closed ifstream. But that's a bit contrived.
Eclipse
I'm all for limiting scope as much as possible :-)
anon
I am all for limiting scope, but adding an unnecessary nested scope is asking for trouble. So maintainer (other than you) may incorrectly see it as unnecessary and remove it thus keeping your file open longer than necessary. Maybe a helper function to restrict scope would be more appropriate?
Martin York
Why an unnecessary scope? Just let the ifstream live in the scope it needs to live to get the job done, and it will be closed just in time - no sooner or later than necessary.
Nemanja Trifunovic
+6  A: 

Do you need to close the file?
NO

Should you close the file?
Depends.

Do you care about the possible error conditions that could occur if the file fails to close correctly? Remember that close calls setstate(failbit) if it fails. The destructor will call close() for you automatically because of RIAA but will not leave you a way of testing the fail bit as the object no longer exists.

Martin York
A: 

No, this is done automatically by the constructor. The only reason you should call it manually, is because the fstream instance has a big scope, for example if it is a member variable of a long living class instance.

Dimitri C.