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

Change MetadataGroup.group variable to HashSet #5556

Merged
merged 1 commit into from
Mar 1, 2023

Conversation

solth
Copy link
Member

@solth solth commented Feb 23, 2023

Fixes #5543

This fixes one part of #4821

@matthias-ronge
Copy link
Collaborator

Could you replace HashSet with TreeSet? This would then also fix #4357. Just a question. You can also first merge this

@solth
Copy link
Member Author

solth commented Feb 27, 2023

I considered using TreeSet instead of HashSet (as you already suggested during our conversation) but ultimately decided against it because of potential performance implications (see point 2.3 on https://www.baeldung.com/java-hashset-vs-treeset).

Although I can see the advantage of always having the metadata in the same order, currently I think avoiding any impacts on performance especially in the metadata editor has to have higher priority.

@solth solth merged commit 5b60965 into kitodo:master Mar 1, 2023
@solth solth deleted the metadata-group branch March 1, 2023 13:33
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.

Improve MetadataGroup
2 participants