Skip to content

Conversation

@decipherer
Copy link
Contributor

@decipherer decipherer commented Feb 8, 2019

This PR offers basically the same change as my last one (#318), but the implementation is slightly different, using subclassing instead of Func<>'s

I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.

@Numpsy Numpsy mentioned this pull request Feb 24, 2019
@piksel piksel self-requested a review May 22, 2019 11:26
@decipherer
Copy link
Contributor Author

I've made some updates to this pull request so that there are no longer any conflicts with the base branch. I've also fixed some of the issues codacy found a while ago.

throw new ArgumentNullException(nameof(buffer));
}

Update(new ArraySegment<byte>(buffer, 0, buffer.Length));
Copy link
Contributor

Choose a reason for hiding this comment

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

Pretty trivial, but just in case it's worth mentioning - is it worth creating an ArraySegment if the version of Update that takes an ArraySegment immediately unwraps it back into an array anyway?

Copy link
Contributor Author

@decipherer decipherer Aug 30, 2019

Choose a reason for hiding this comment

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

I did some benchmarks directly calling Proxy.Append, instead of calling the other version of Update. Unfortunately I did not see any speed benefit. But I do think the code is a bit inconsistent, there are 2 Update methods where Proxy.Append is called directly, then there is a 3rd Update method that calls one of the other 2 Update methods.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose that handling these sort of differences in where Span and friends can be useful, though I guess the extra dependency they have in versions of .Net prior to the latest would be unwanted?

@Numpsy Numpsy mentioned this pull request Sep 22, 2020
HowToDoThis added a commit to HowToDoThis/SharpZipLib that referenced this pull request Nov 21, 2020
@piksel
Copy link
Member

piksel commented Nov 22, 2020

See #306

@piksel piksel closed this Nov 22, 2020
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.

4 participants