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

Allow the user the get the current number of bytes written to the wrapped writer. #12

Closed

Conversation

PaulGrandperrin
Copy link

In a project I'm working on, I need to know an estimation of how much bytes have already been written to decide when to call Encoder.finish().
I need that to be able to create evenly sized compressed blocks in a bigtable like implementation.

Allow the user the get the current number of bytes written to the wrapped writer.
@bozaro
Copy link
Owner

bozaro commented Mar 20, 2016

I do not like this change:

  • Adding Encoder.get_written_size also involves adding Decoder.get_readed_size;
  • This functionality can be added without lz4-rs modification by code like:
struct Wrapper<W: Write> {
    w: W,
    written_size: usize,
}

impl<W: Write> Wrapper<W> {
    pub fn get_written_size(&self) -> usize {
        self.written_size
    }
}

impl<W: Write> Write for Wrapper<W> {
    fn write(&mut self, buffer: &[u8]) -> Result<usize> {
        self.written_size += buffer.len();
        self.w.write(buffer)
    }

    fn flush(&mut self) -> Result<()> {
        self.w.flush()
    }
}

@PaulGrandperrin
Copy link
Author

I didn't thought of that but I think it won't work.
When you build an Encoder, the Writer is moved inside it and is therefore inaccessible until you get it back from finish().

As a matter of fact, the Writter I'm using already implements the method len() which I could have used if it wasn't moved...

@bozaro
Copy link
Owner

bozaro commented Mar 20, 2016

Sounds reasonable.
I will add this feature fore Encoder/Decoder in few days.

@PaulGrandperrin
Copy link
Author

Great, thank you! I didn't felt like using the C bindings directly :-)

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.

None yet

2 participants