search

jaime-olivares / zipstorer

A Pure C# Class to Store Files in Zip
MIT License
183 stars 63 forks source link

Close only streams which where opened internally #6

Closed dguder closed 7 years ago

dguder commented 8 years ago

Pull request for #1

jaime-olivares commented 8 years ago

Hi @dguder, thanks for posting. I am looking at your approach and it seems you are doing an assumption based on the factory method (file or stream). I think this won't cover all the use cases and will even generate a backwards compatibility issue, considering this library is already widely used.

What I can do is to add an optional flag in all the factory methods, lets call it _preventClosing (default=false), so the user can be specific about how to handle the stream.

Does it sound reasonable?

dguder commented 8 years ago

Hi @jaime-olivares, you are right, the backwards compatibility is an issue. Under this aspect your proposal is valid. But in general the one who openend the stream should always close it. And it is more or less a _preventDisposing than _preventClosing. I'm not very happy with both solutions. But I don't have a good idea how to arrange this.

For a filestream it is totally ok, to dispose it when you closed the zip. But for an external defined memory stream, it should not be disposed. It could be either sent over the wire without writing a temp file. It could be also added to another file. Maybe zip in zip.

Maybe the Close() should not dispose the stream but the Dispose(). If this is done by Dispose() then the function Close() must detect if it was called already and then skip writing CentralDirImage and EndRecord again.

Another solution could be to keep it like it is and I will use my slightly modified version.

So far dguder

jaime-olivares commented 7 years ago

Closing for now as there is not a clear consensus