Thread: Should you manually close an fstream?

  1. #16
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    Quote Originally Posted by King Mir View Post
    This is not always possible. consider:
    Code:
    {
      ifstream file1 ("file1.txt");
      //read from file1 here
      ifstream file2 ("file2.txt");
      //use file1 and file2 here
      file1.close();
      //keep using file 2 here
    }
    Now you could do this:
    Code:
    ifstream file2
    {
      ifstream file1 ("file1.txt");
      //read from file1 here
      file2.open("file2.txt");
      //use file1 and file2 here
    }
    //keep using file 2 here
    But that vialates RAII too.

    Streams are the example, but they are not the only resouse that could encounter this problem.

    Although this problem isn't that common, I don't think it's invariably a design problem when it does come up.
    Sorry, but I agree with whiteflags that your example is contrived, since you have left out any consideration of the lifetime of file2. Your examples can easily be reorganised as
    Code:
    void FunctionToGiveAnAdditionalContainingScope()
    {
         ifstream file2;
    
        {    // begin the scope as per your example
             ifstream file1 ("file1.txt");
              //read from file1 here
              file2.open("file2.txt");
              //use file1 and file2 here
         }
            // use file2 here
    }    // file2 stream's destructor invoked here, so the stream will be closed.
    I would probably organise the enclosed scope into its own function as well, but it is not technically necessary.

    You are certainly correct that programs don't use one resource at at time, and release each resource before using another one. However, every resource is associated with a lifetime, even if lifetimes of distinct resources overlap. There is a point before which the resource is not needed. There is a point at which it is no longer needed. Those two points can be associated with the begin and end of a scope.

    Even if a resource has infinite lifetime, it can be associated with a scope. For example, a program that never terminates.
    Code:
    int main()
    {
         Resource some_resource;
         bool reality_exists = true;
    
         do
         {
                // use some_resource
         }
         while (reality_exists);
    }
    Short of some Armageddon event which (as far as the program is concerned) destroys all reality, some_resource will continue to exist. But it is still associated with a scope.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  2. #17
    Registered User
    Join Date
    Apr 2006
    Posts
    2,149
    The first bit of code you posted is in violation of RAII ideals, because you're using 2-stage construction, which is about as bad as 2-stage destruction ala close. It's more intuitive to do it that way in some contexts, I agree, but I don't see why it is idiomatically or generally better. Why should 2 stage construction be preferred over 2-stage destruction?
    It is too clear and so it is hard to see.
    A dunce once searched for fire with a lighted lantern.
    Had he known what fire was,
    He could have cooked his rice much sooner.

  3. #18
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    This is not actually a 2-stage destruction. Closing a stream is a change of stream state, not a phase of destruction.

    Technically, I could have changed the first code sample to
    Code:
    void FunctionToGiveAnAdditionalContainingScope()
    {
         ifstream file2("file2.txt");
     
        {    // begin the scope as per your example
             ifstream file1 ("file1.txt");
              //use file1 and file2 here
         }
            // use file2 here
    }    // file2 stream's destructor invoked here, so the stream will be closed.
    but decided I didn't want to get into the debate if you were to adjust the example so the filename for file2 became derived from data in file1.

    Incidentally, I just realised I could have also used the rule that "automatic destruction of objects due to scope is in the reverse order of their construction" and produced this
    Code:
    void FunctionToGiveAnAdditionalContainingScope()
    {
         ifstream file1 ("file1.txt");
            // read data from file1
         ifstream file2(some_filename_derived_from_data_read_from_file1);
           //use file1 and file2 here
    
         file1.close();    //   this is strictly optional
    
          // use file2 here
    }    // file2 stream's destructor invoked here, then file1's destructor, so the streams will be closed if not already closed.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  4. #19
    Registered User
    Join Date
    Apr 2006
    Posts
    2,149
    Quote Originally Posted by grumpy View Post
    This is not actually a 2-stage destruction. Closing a stream is a change of stream state, not a phase of destruction.
    Closing a stream is resource release. Opening a stream is resource acquisition. Delaying opening a stream violates Resource Acquisition Is Initialization, trivially. If RAII is to imply that resource release is destruction, which it reasonably should, then releasing a resource, such as by closing a stream, violates RAII too.

    Yet it's not a fault of design that resource lifetimes are sometimes intertwined like that.
    Incidentally, I just realised I could have also used the rule that "automatic destruction of objects due to scope is in the reverse order of their construction" and produced this
    Code:
    void FunctionToGiveAnAdditionalContainingScope()
    {
         ifstream file1 ("file1.txt");
            // read data from file1
         ifstream file2(some_filename_derived_from_data_read_from_file1);
           //use file1 and file2 here
    
         file1.close();    //   this is strictly optional
    
          // use file2 here
    }    // file2 stream's destructor invoked here, then file1's destructor, so the streams will be closed if not already closed.
    Closing file1 is not strictly optional, since closing a file can effect whether other programs can open the file, and the number of other files the program is allowed to open by the OS. In general, resources should be released as soon as they are no longer needed, and no later.
    It is too clear and so it is hard to see.
    A dunce once searched for fire with a lighted lantern.
    Had he known what fire was,
    He could have cooked his rice much sooner.

  5. #20
    the hat of redundancy hat nvoigt's Avatar
    Join Date
    Aug 2001
    Location
    Hannover, Germany
    Posts
    3,130
    Most bugs are common misunderstandings. If you do something smart, make sure all people in the project are at least as smart as you. So if you aren't the dumbest member of your team, don't try smart things. You aren't a team player. Maybe other people aren't actually as smart as you. Or maybe they are the worlds top genius, but their language of choice is Java or PHP or something else. The best solution to your problem at hand is the simplest solution that solves the problem.

    Think about somebody else reading your code. And maybe, he's just a normal guy and doesn't know the C++ standard by heart.

    Code:
    std::string read_smart()
    {
    	// ok, he has a text
    	std::string result;
    
    	// what's this? did he delete an if and forgot the braces?
    	{
    		// wtf is an if stream? does it stream if... if what? maybe that's why the if was missing?
    		std::ifstream ifs("somefile.txt");
    
    		// and now he bitshifts the maybe streaming thing by the text?
    		ifs >> result;
    	}
    
    	// somehow, the file landed in here... I'm totally confused.
    	return result;
    }
    
    std::string read_simple()
    {
    	// ok, he has a file.
    	std::ifstream file;
    
    	// and it's contents
    	std::string file_content;
    
    	// he opens the file
    	file.open("somefile.txt");
    
    	// and bitshifts? hm, somehow this line puts the file into the file_content... C++ is wierd
    	file >> file_content;
    
    	// he closes the file
    	file.close();
    
    	// now that was easy to understand
    	return file_content;
    }
    hth
    -nv

    She was so Blonde, she spent 20 minutes looking at the orange juice can because it said "Concentrate."

    When in doubt, read the FAQ.
    Then ask a smart question.

  6. #21
    Lurking whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    9,613
    Yeah sometimes I see code like this, and have a little chuckle:

    Code:
    ifstream file("somefile.txt");
    
    string fileContent;
    
    if ( file.is_open() ) 
    {  // start if
        
       file >> fileContent;
    
    } //end if
    
    return fileContent;
    But the comments there can be useful, say if the block stretches past what you can see on the screen. I think a comment explaining the extra block you put in is required, because it could answer many questions about it.

  7. #22
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by nvoigt
    Or maybe they are the worlds top genius, but their language of choice is Java or PHP or something else. The best solution to your problem at hand is the simplest solution that solves the problem.

    Think about somebody else reading your code. And maybe, he's just a normal guy and doesn't know the C++ standard by heart.
    My problem with that argument is that if you want to program for the benefit of such a person who lacks knowledge of a given library component that is used and does not bother to learn about it before jumping to a conclusion based on his/her existing knowledge of components from a different language or library, then you're going to have a pretty hard time doing anything that is idiomatically C++.

    For example, your example has a latent bug: what happens if an exception is thrown before the close member function is called, e.g., an exception propagated from some std::string related function that is called when reading into the string variable? Clearly, we should catch the exception and then close the file, otherwise the file would not be closed when the ifstream object is destroyed. Oh, and it looks like C++ is deficient in its exception handling: a finally clause would help here. RAII? What's that? C++ is weird.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  8. #23
    Registered User C_ntua's Avatar
    Join Date
    Jun 2008
    Posts
    1,853
    Quote Originally Posted by King Mir
    Closing file1 is not strictly optional, since closing a file can effect whether other programs can open the file, and the number of other files the program is allowed to open by the OS. In general, resources should be released as soon as they are no longer needed, and no later.
    In either ways the file is closed, no?

    I understand that leaving a file open is bad where not deallocating memory as soon as possible usually not. So you can consider that a file should need to manually be handled. But the alternative to that is to teach programmers to use a file strictly when they need it. So you can start your program as
    Code:
    std::string current_filename;
    std::ifstream current_stream;
    std::string fbuffer;
    ....
    //a 100 lines
    ...
    current_stream.open(current_filename);
    ...
    //read into fbuffer
    ...
    //a 100 lines
    ...
    current_stream.close();
    ....
    ...
    but I would prefer if you designed your program as

    Code:
    std::string current_filename;
    std::string fbuffer;
    ....
    //a 100 lines
    ....
    {
      std::ifstream current_stream(current_filename);
      ...
      //read into fbuffer
      ...
    }
    ...
    ...
    because then I know exactly where the file is used and how long the file is open. The compiler doesn't keep track of open() and close() statements to throw a "fstream never closed" warning or "reading from fstream while it was already closed".

  9. #24
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    Quote Originally Posted by King Mir View Post
    Closing a stream is resource release. Opening a stream is resource acquisition. Delaying opening a stream violates Resource Acquisition Is Initialization, trivially. If RAII is to imply that resource release is destruction, which it reasonably should, then releasing a resource, such as by closing a stream, violates RAII too.
    The thing is, there are two types of resource here.

    The first is the ifstream itself, which is all our code should worry about. As long as we manage the lifetime of that ifstream appropriately, we have met our responsibility for resource management.

    The second is the resources that the ifstream itself manages. Our code should not need to worry about managing those ifstream-internal resources at all, and if we need to that is a design problem with the ifstream class. If we can rely on an ifstream appropriately managing its internal resources, and we manage the lifecycle of the stream object, we need do no more.

    Quote Originally Posted by King Mir View Post
    Yet it's not a fault of design that resource lifetimes are sometimes intertwined like that.
    It is unnecessary effort for a programmer to worry about managing a resource that is internal to the ifstream class though.

    I agree that resource lifetimes can overlap. However, if managing the lifecycle of one resource (an ifstream) completely manages the lifecycle or state of some other resource (the ifstream internals) then we don't need to manage that anything related to that "other resource".

    Quote Originally Posted by King Mir View Post
    Closing file1 is not strictly optional, since closing a file can effect whether other programs can open the file, and the number of other files the program is allowed to open by the OS. In general, resources should be released as soon as they are no longer needed, and no later.
    As far as the user of file1 is concerned, closing the stream is optional, since we manage its lifecycle.

    The fact that we can deduce that ifstream manages a resource internally does not mean we need to manage that internal resource.

    In any event, you've just made your example more contrived. On one hand, you have a need for both streams to be open. But you need to close one of them to conserve resources, before you've finished with the other? Any design which is that fragile has a good chance of breaking before you can close file1.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  10. #25
    the hat of redundancy hat nvoigt's Avatar
    Join Date
    Aug 2001
    Location
    Hannover, Germany
    Posts
    3,130
    Quote Originally Posted by laserlight View Post
    My problem with that argument is that if you want to program for the benefit of such a person who lacks knowledge of a given library component that is used and does not bother to learn about it before jumping to a conclusion based on his/her existing knowledge of components from a different language or library, then you're going to have a pretty hard time doing anything that is idiomatically C++.

    For example, your example has a latent bug: what happens if an exception is thrown before the close member function is called, e.g., an exception propagated from some std::string related function that is called when reading into the string variable? Clearly, we should catch the exception and then close the file, otherwise the file would not be closed when the ifstream object is destroyed. Oh, and it looks like C++ is deficient in its exception handling: a finally clause would help here. RAII? What's that? C++ is weird.
    I agree that if we dumb down C++ enough then we end up using "C with classes" and nobody wants that. C++ has some neccessarily complicated ways of getting stuff done. However, I don't think this is one of it. If the goal is to close a file, then it really doesn't get simpler than file.close(); If someone opens up an extra scope because he knows that losing scope will call close behind the scenes, then I consider this unneccessarily complicated.

    As far as I can see, both of my example codes, if an exception is thrown, will destruct and therefor close the file. Both file streams are contained in a scope that would be left with an exception. I'm not expecting anyone not experienced in C++ to write C++. But it should be as easy as possible to read. If I have the choice between good and readable, and great and unreadable code, I'd pick the former.
    hth
    -nv

    She was so Blonde, she spent 20 minutes looking at the orange juice can because it said "Concentrate."

    When in doubt, read the FAQ.
    Then ask a smart question.

  11. #26
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by nvoigt View Post
    I agree that if we dumb down C++ enough then we end up using "C with classes" and nobody wants that. C++ has some neccessarily complicated ways of getting stuff done. However, I don't think this is one of it. If the goal is to close a file, then it really doesn't get simpler than file.close(); If someone opens up an extra scope because he knows that losing scope will call close behind the scenes, then I consider this unneccessarily complicated.
    While I agree with you at some points, I wouldn't consider an extra scope unnecessarily complicated. This idiom is all the rage right now and is being incorporated into other languages. C# already has the using statement which is just using + a scope, and there are talks of implementing something similar into Java.
    I consider an extra scope something of the norm, rather than something of the odd. If a programmer does not know of this idiom, then I dare say they have failed to grasp an important concept which they should know. We should not make brittle code (or adapt our code) to these programmers. They should adapt to the normal idioms used in the programming world.
    Well, that's my opinion on the matter, anyway.
    Last edited by Elysia; 12-10-2012 at 05:41 AM.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  12. #27
    the hat of redundancy hat nvoigt's Avatar
    Join Date
    Aug 2001
    Location
    Hannover, Germany
    Posts
    3,130
    C# already has the using constructor which is just using + a scope
    The main difference and my only problem with the same idiom in C++ is that in C# the using construct is a very clear communication of intent. I'm using( ... ) a specific variable and I want it gone when the block ends. In C++ I have a scope... and the programmer is left guessing which variable in the scope I wanted to be gone... if any. I have seen a lot of useless scopes (deleted ifs with braces left, for example) that were the result of laziness or sloppy programming so I'd prefer clean communication.
    hth
    -nv

    She was so Blonde, she spent 20 minutes looking at the orange juice can because it said "Concentrate."

    When in doubt, read the FAQ.
    Then ask a smart question.

  13. #28
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    I very rarely, in practice, use a naked pair of braces to introduce a new scope. Except in occasional forum discussions ....

    Generally speaking, if I want to introduce an additional scope, I'll create a new function to provide it. Particularly if I can give a name to the function that communicate what it achieves.

    If using C++-11 and feeling like doing something a little funky, I might even do it with a lambda
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  14. #29
    Registered User C_ntua's Avatar
    Join Date
    Jun 2008
    Posts
    1,853
    Quote Originally Posted by nvoigt View Post
    I agree that if we dumb down C++ enough then we end up using "C with classes" and nobody wants that. C++ has some neccessarily complicated ways of getting stuff done. However, I don't think this is one of it. If the goal is to close a file, then it really doesn't get simpler than file.close(); If someone opens up an extra scope because he knows that losing scope will call close behind the scenes, then I consider this unneccessarily complicated.
    I agree if you want to close a file you call close(). When you want to open it you call open(). Where I disagree is that if you just want to read from a file you need to manually open(), read() and close(). You also don't need to explain to someone how resources are allocated/deallocated. You just explain that the file follows the same lifecycle as fstream. They should be familiar with the scope of a variable so this is not something different.

    Note that the only reason you bother adding brackets is because you are adding the requirement "close the file as soon as possible". If the requirement was "don't forget to leave the file open" then you don't even need to worry about the brackets. Let us assume that I have the "don't leave the file open" requirement. If the examples I read advice to use close() I assume it is mandatory. So I will add on my project the additional step of making sure all the fstream() used in all files of my project call close(). This is an additional cost for my project.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. when parent process close, how to close the child?
    By omega666 in forum C Programming
    Replies: 4
    Last Post: 04-06-2011, 12:23 AM
  2. manually sorting cards
    By cyberfish in forum A Brief History of Cprogramming.com
    Replies: 33
    Last Post: 05-16-2008, 09:09 AM
  3. Manually adding controls to a wnd and their functions.
    By earth_angel in forum Windows Programming
    Replies: 4
    Last Post: 08-22-2005, 06:26 PM
  4. uhm... calling a destructor manually
    By *ClownPimp* in forum C++ Programming
    Replies: 2
    Last Post: 09-11-2002, 02:37 PM
  5. Manually programming Press any key in vis C++
    By vanceypants in forum C++ Programming
    Replies: 1
    Last Post: 06-06-2002, 04:23 AM