Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

try with resources #5

Merged
merged 2 commits into from Jul 17, 2020
Merged

try with resources #5

merged 2 commits into from Jul 17, 2020

Conversation

elharo
Copy link
Contributor

@elharo elharo commented Jul 16, 2020

@@ -300,8 +293,6 @@ private void populateFile( File file ) throws IOException
try ( FileOutputStream outputStream = new FileOutputStream( file ) )
{
outputStream.write( data.getBytes() );
outputStream.flush();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think the .flush() is guaranteed by try-with-resources... but perhaps by not wrapping FOS in a BufferedOutputStream then that's not a problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

close also flushes

Copy link
Contributor

@pzygielo pzygielo Jul 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @roxspring is right about FOS not wrapped in BOS not being problem for flushing or not. But I'm not sure that close also flushes. There is flush called in FOS.finalize but close inherited from OutputStream is no-op (immaterial), as FOS.close doesn't seem to call flush neither.
On the other hand close of BufferedOutputStream flushes indeed.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling flush just before close is generally useless cause close ensures all data have been written.
That said we are speaking of a FileOutputStream where flush() is a noop so it is even safer to drop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants