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

Why return a ReadCloser for reading, but not a WriteCloser for writing? #74

Open
leijurv opened this issue Nov 13, 2019 · 3 comments
Open

Comments

@leijurv
Copy link

leijurv commented Nov 13, 2019

Is there some reason why I shouldn't treat the compression step as a WriteCloser? Why does that return a public struct instead of an interface, like how decompression does?

Thanks!

@Viq111
Copy link
Collaborator

Viq111 commented Nov 13, 2019

Hi @leijurv!

Yes we could return an io.WriteCloser, though it would mean the public attribute CompressionLevel would not be exposed anymore

If you want to use a io.WriteCloser, you can just do a:

var zstdWriter io.WriteCloser = NewWriter(w)

If you are talking about consistency, it's just to provide the attribute above ^ (for now, for future, we could also provide additional methods / attributes)

Hope this helps

@leijurv
Copy link
Author

leijurv commented Nov 14, 2019

I'm not sure I understand. Why is the CompressionLevel public? Am I permitted to change it in the middle of writing?? (if so, that's awesome). If it's unchangeable then why public?

@Viq111
Copy link
Collaborator

Viq111 commented Dec 3, 2019

It is indeed only public for read (get the value) but not for write (editing it won't change anything as the zstd_ctx is already created with the initial value: https://github.com/DataDog/zstd/blob/1.x/zstd_stream.go#L68)

That would definitely make sense to convert it to a method though to make the read-only explicit.
Since this is technically an API change though, we'll need to wait to switch to a major (2.x)

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

No branches or pull requests

2 participants