Consider standardizing the Read() Method in ZInputStream

[kami-poi]

I'm using this project in an ASP.net core application.
To process data I should use some async Stream methods. In Stream.ReadAsync it will call Read method in a loop and expect 0 as return when nothing can read. When -1 returned, it will throw a ArgumentOutOfRangeException and block the processing of request. So I hope you can standardization the return value to make it matching C# Language Specification. Tkanks.

[AraHaan]

Well I do offer an helper class too called MemoryZlib that can abstruct the streams, however I made it optional, I have been recently thinking if I should merge the ZInput and ZOutput Stream classes into an single stream class and yeet the original ZStream class for that one.

Also those helpers have overloads taking in byte arrays, an optional output of the adler32 hash of the compressed output, and also other overloads taking in an stream for input that could be used as output. There are other overloads like if you wanted to specify the full path to a file, the compression level (if compressing), and optionally output the adler32, while outputting the data to a stream or a byte array.

Note: The Stream overloads on the helper class mentioned above was added after the latest stable, it can be consumed by a prerelease build but I have yet to set up prerelease deployments again to deploy the latest bits and changes (currently there was minor code fixups to syntax from using newer c# language features in older Frameworks and a reduction in used frameworks from 19 TFM’s (Target Frameworks) down to 4 which greatly reduced build time.

But I do agree I think I should implement asnyc into the streams, and possibly async overloads into the helpers as well too.

[AraHaan]

Also forgot to say that Read is implemented inside of the input stream class only if I remember right so far, however I think by unifying them I would eliminate the need to have special input and output streams and have the unified stream be ZlibStream (since ZStream is already taken and should only be an internal symbols to help with compression / decompression and should not be used directly even if that class is public as in the future I might make it either vanish completely (into the ZlibStream class) or as an internal class).

[kami-poi]

Thanks for replying.
I found that I accidentally wrote ZOutputStream in the title. All the problems just happend at ZInputStream. Sorry!

Merge ZInputStream and ZOutputStream into ZlibStream is a good idea. It is simple and clear, and similar to the compressed stream provided by .Net. People can easily use it just like use those official streams.

The problem has solved too. I just copy your code and modified return value to make it work. When your new stable release comes, I will switch back to your project. Thanks.

[AraHaan]

Alright so I have made a few classes that did not expose any of it’s members that was public as internal, nuked ZStreamException for the other exceptions I made as they make it easier to differentiate if the exception was caused by compressing, or decompressing, and then exposed making an adler32 hash (as MemoryZlib.ZlibGetAdler32(byte[] data, int index, int length))as people might want to use that namely someone like me might want to hash the original data before compressing it and store it somewhere where after decompressing would trap any possible corruption when decompressing which would be nice, or could be used to get the hash of the compressed data.

Will work on unifying the streams too.

Also ZlibCost.Version() => MemoryZlib.ZlibVersion() so I can nuke that small file as well too.

[AraHaan]

@kami-poi think Write on the unified stream should be used only for compression mode, while Read is when not compressing?

I been thinking on this a lot as it seems currently on the separate streams that Read could be used for both compressing and decompressing, same for write and I think that could be changed to where only Read is for decompressing, and Write for compressing.

Wait crap then it would break CopyTo where you copy from the source stream into the Unified stream as it calls Write() on both compressing and decompressing.

I guess if the person place in the actual data into the stream param ctor of the unified stream so it would already have the data and then use CopyTo to copy the contents from the unified stream to the output stream (while it also decompresses the data).

I guess if I went that route I would need to catch when someone tries to use Write() for decompressing to tell them to instead pass the compressed data into the ctor and then use CopyTo to copy from the unified stream to the output stream.

I also think it should be wise to also embed code examples in the docs as well to show the user how to use the unified streams then as well.

[AraHaan]

Alright part 1 is now in #139.

[kami-poi]

I know what you mean. Microsoft suggets use compression stream like this. So it won't call Write() both compressing and decompressing.

Support both Read() and Write() is possible. But it will bring inconsistent behavior to the same mathod, just because they are in different compression mode. In some case it may cause some unexcepted behavior. And people may confuse what they are doing. So I still support only enable one method for compression or decompression, like what Microsoft do.

[AraHaan]

So in the case of that it seems what microsoft does is ensures that decompress is in Read() while Compression enables Write() but they also check the underlying stream if it can read or write as well too before setting them to true.

[AraHaan]

part 2 is now in https://github.com/Elskom/zlib.managed/pull/140.

[AraHaan]

Alright so I think I should decouple the dependency references from needing the parent repository and then set up for unit testing before considering pushing a new release.