6

Should I close a file when it wasn't able to open?

Should I write this:

std::ifstream file(DATA_PATH);
if (!file.good()) //File doesn't exist
{
    //do something
}
else //file exists
{
    //do something
    file.close();
}

Or should I write:

std::ifstream file(DATA_PATH);
if (!file.good()) //File doesn't exist
{
    //do something
}
else //file exists
{
    //do something
}
file.close();
giladk
  • 148
  • 3
  • 13
  • Related: [Do I need to manually close an ifstream?](https://stackoverflow.com/q/748014/2602718) (spoiler alert: **no**) – scohe001 Jan 22 '19 at 20:53

2 Answers2

5

No, that's not necessary to be done explicitly. (File) streams are closed when going out of scope implicitly always.

The close() function of a std::iostream() also is an idempotent operation, and never will harm the streams state beyond the stream gets closed (or never was successfully opened).

πάντα ῥεῖ
  • 1
  • 13
  • 116
  • 190
  • Sorry, can't help it, but to quote from Friends: "`Monica`: Hey Joey, what would you do if you were omnipotent? `Joey`: Probably kill myself. `Monica`: Excuse me? `Joey`: Hey, if little Joey's dead, then I've got no reason to live. `Ross`: Joey, omnipotent. `Joey`: You are? I'm so sorry." – SergeyA Jan 22 '19 at 21:21
  • @SergeyA _Idempotent_ and _omnipotent_ mean quite different things. – πάντα ῥεῖ Jan 23 '19 at 00:08
  • Well, I am aware :) Just couldn't help the joke – SergeyA Jan 23 '19 at 14:07
0

You DO NOT need to call close() if the stream fails to open. On the other hand, it is not harmful to do so, either.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • To be fair, you don't need to call `close` in either case. I'm not even sure why this function exists – Quentin Jan 22 '19 at 20:55
  • 1
    @Quentin Do you need to `close` before `open`ing a different file? Might be a use case for it, to reuse an instance. Though you could just construct another instance and move assign it like any other value type, but I'm not sure what the overhead for that might be. – François Andrieux Jan 22 '19 at 20:58
  • 4
    @Quentin Well, depends on the code structure and workflow. There are certainly use cases where it makes sense to call close() independently of the variable's scope. – πάντα ῥεῖ Jan 22 '19 at 20:59
  • @FrançoisAndrieux "*Though you could just construct another instance and move assign it like any other value type*" - `open()`/`close()` predate the introduction of move semantics. – Remy Lebeau Jan 22 '19 at 21:10
  • 1
    @RemyLebeau Good point. I sometimes forget that dark age existed. – François Andrieux Jan 22 '19 at 21:11
  • @Quentin At least theoretically there is another use case: close() could throw an exception on error - whereas the destructor simply can't. If there is any error whilst closing a file, failbit will be set. Iff your stream has enabled exceptions (not the default case!) an exception will be thrown for errors that happen during closing a file Such errors are simply ignored during destructors! – MFH Jan 23 '19 at 01:57
  • @MFH fair enough. – Quentin Jan 23 '19 at 09:42