gh-152140: Replace _Extra class with ZipFile._strip_extra_fields()#152141
Open
danny0838 wants to merge 3 commits into
Open
gh-152140: Replace _Extra class with ZipFile._strip_extra_fields()#152141danny0838 wants to merge 3 commits into
_Extra class with ZipFile._strip_extra_fields()#152141danny0838 wants to merge 3 commits into
Conversation
…elds()` The `_Extra` class was over-engineered. Its only active usage was its `strip()` method, called by `ZipFile._write_end_record()` to provide the stripping logic for ZIP64 fields. The context-dependent nature of extra fields also made it difficult to be reused by `_decodeExtra()` or other methods efficiently. Additionally, its `split()` method called `_Extra` directly rather than utilizing `cls`, which was a suboptimal pattern that hindered extensibility. Remove the `_Extra` class entirely and reimplement it as a private static method `_strip_extra_fields()` that processes a bytearray inside `ZipFile`, positioned directly beneath its caller. This eliminates dead and suboptimal code, achieves clean encapsulation and code locality, and improves performance by avoiding temporary class allocations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
_Extraclass was over-engineered. Its only active usage was itsstrip()method, called byZipFile._write_end_record()to provide the stripping logic for ZIP64 fields. The context-dependent nature of extra fields also made it difficult to be reused by_decodeExtra()or other methods efficiently. Additionally, itssplit()method called_Extradirectly rather than utilizingcls, which was a suboptimal pattern that hindered extensibility.Remove the
_Extraclass entirely and reimplement it as a private static method_strip_extra_fields()that processes a bytearray insideZipFile, positioned directly beneath its caller. This eliminates dead and suboptimal code, achieves clean encapsulation and code locality, and improves performance by avoiding temporary class allocations.Additionally, bind this method as a class property in the tests to simplify the code.
zipfilemodule #152140