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

Refactor KiwiJars#readValuesFromJarManifest to read the manifest once #1199

Closed
sleberknight opened this issue Sep 14, 2024 · 0 comments · Fixed by #1200
Closed

Refactor KiwiJars#readValuesFromJarManifest to read the manifest once #1199

sleberknight opened this issue Sep 14, 2024 · 0 comments · Fixed by #1200
Assignees
Labels
refactoring Code refactoring
Milestone

Comments

@sleberknight
Copy link
Member

The KiwiJars#readValuesFromJarManifest(ClassLoader classLoader, Predicate<URL> manifestFilter, String... manifestEntryNames) method reads the manifest for every one of the manifestEntryNames. So, if five names are provided, it reads the manifest five separate times, doing way more work (and I/O) than it should. This method should only read the manifest file once.

@sleberknight sleberknight added the refactoring Code refactoring label Sep 14, 2024
@sleberknight sleberknight added this to the 4.4.1 milestone Sep 14, 2024
@sleberknight sleberknight self-assigned this Sep 14, 2024
sleberknight added a commit that referenced this issue Sep 19, 2024
* Refactor readValuesFromJarManifest so it only reads the Manifest once.
* Add Nullable annotation to the Predicate in several methods.
* Change singular variable name 'value' in KiwiJarsTest to plural
  'values' in tests of readValuesFromJarManifest, so that it is clearer
  that the result contains multiple values.

Closes #1199
sleberknight added a commit that referenced this issue Sep 19, 2024
* Refactor readValuesFromJarManifest so it only reads the Manifest once.
* Add Nullable annotation to the Predicate in several methods.
* Change singular variable name 'value' in KiwiJarsTest to plural
'values' in tests of readValuesFromJarManifest, so that it is clearer
that the result contains multiple values.

Closes #1199
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Code refactoring
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant