close
Skip to content

feat: [FLAC] drop io.Closer element in Stream and Encoder types#70

Merged
mewmew merged 3 commits into
mewkiz:masterfrom
zalgonoise:feature/find-io-closer-in-reader-interface
May 8, 2024
Merged

feat: [FLAC] drop io.Closer element in Stream and Encoder types#70
mewmew merged 3 commits into
mewkiz:masterfrom
zalgonoise:feature/find-io-closer-in-reader-interface

Conversation

@zalgonoise
Copy link
Copy Markdown
Contributor

The io.Closer type is taken only if the underlying io.Reader (for Stream) and io.Writer (for Encoder) implements io.Closer.

This being the case, there is no point in storing it as an additional element in these data structures. It both makes them simpler and shorter (minus one pointer type, considering that interfaces are stored as pointer types), and it also makes the code a bit more readable.

Another detail is the change in the encoder frames logic, which initializes a bitio.Writer which wraps our Encoder's writer, and the Encoder's closer is replaced with the bitio.Writer instance -- however, bitio.Writer.Closer does not close the underlying writer.

Copy link
Copy Markdown
Member

@mewmew mewmew left a comment

Choose a reason for hiding this comment

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

LGTM.

@mewmew
Copy link
Copy Markdown
Member

mewmew commented May 8, 2024

Hi @zalgonoise!

Thanks for submitting the PR. I agree that the code looks cleaner in the way you wrote it.

Happy to merge : )

Cheers,
Robin

@mewmew mewmew merged commit c38ea50 into mewkiz:master May 8, 2024
@zalgonoise zalgonoise deleted the feature/find-io-closer-in-reader-interface branch May 8, 2024 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants