Initialize is_open value in ReadFileBuffer. #254
Conversation
Address comments in issue QMCPACK#253. - initialize the value of is_open to false by default - Update the filename in the APP_ABORT messages - change get_file_length to be a member function.
|
In open_file, set is_open to false if an open attempt fails. |
| @@ -86,10 +86,12 @@ class ReadFileBuffer | |||
| char *cbuffer; | |||
| std::ifstream *fin; | |||
| Communicate *myComm; | |||
| int get_file_length(std::ifstream *f); | |||
There was a problem hiding this comment.
Add const if this routine doesn't touch member variables.
There was a problem hiding this comment.
How about remove the argument and directly operates on fin?
There was a problem hiding this comment.
How to write get_file_length was mentioned in the bug report on OO vs. functional. Yes, it could operate directly on fin and write to length. However, I think that makes it harder to understand in isolation vs. using a more functional style.
|
The behavior for re-using this class is undefined, since it's currently not used that way.
I would be inclined to go for (1), because it would simplify the usage. |
|
The fact that open_file operates differently from other standard "open" provided by languages is a potential trap.
|
If a file has been opened using ReadFileBuffer, close it and clear the buffer before opening the new file. Add a unit test for the re-open case. Make get_file_length const, as it does not modify the class.
Address comments in issue #253.