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

fix write operation #16

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fix write operation #16

wants to merge 4 commits into from

Conversation

fubai
Copy link

@fubai fubai commented Oct 18, 2016

No description provided.

@louiszuckerman
Copy link
Collaborator

Hi Allen,

Thank you for your contribution. I'm a bit puzzled by this change, and would like to get more information to understand it better.

How does the current code cause problems? Can you show me how to reproduce the issue that this change fixes?

Also, I'd like to improve the test suite to cover this issue. Currently the tests fail with your changes. We'll need to fix the existing test that is failing, and write a new test to cover the problem you've identified. The new test should fail with the old code, and pass with your new changes.

We can work together on the test cases, but I need to better understand what the problem is that you're solving here. I look forward to hearing more about it.

Thanks again!

-louis

@fubai
Copy link
Author

fubai commented Oct 19, 2016

Hi semiosis,

try (OutputStream output = Files.newOutputStream(path, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) {
    int len;
    byte[] buffer = new byte[8192];
    while ((len = input.read(buffer, 0, 8192)) > 0) {
        output.write(buffer, 0, len);
    }
    output.flush();
}

here I'm using buffer to transfer byte from input to output , where int len is the actual length of input file, so we should write this length of byte to output , not the whole byte everytime.

e.g.

input provide 8200 bytes , so we got buffer filled with 8192 the first time in while loop , with 8 bytes the second time , what we should write is 8192 + 8 , not 8192 *2 as old code.

Copy link
Collaborator

@louiszuckerman louiszuckerman left a comment

Choose a reason for hiding this comment

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

Haven't tested this but it looks good to me.

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