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

Replace large switch statement with a lookup table. #212

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

twalmsley
Copy link
Collaborator

Closes #211

For discussion, but this looks better to me than the original switch statement. Feel free not to merge if you disagree :-)

Hopefully it will also be more efficient since it removes a large cascading if...else... construct (depending on compiler optimisations of course).

@twalmsley twalmsley self-assigned this Apr 23, 2024
@twalmsley twalmsley linked an issue Apr 23, 2024 that may be closed by this pull request
@@ -136,7 +140,7 @@ private static <T extends Thing> java.lang.Class<T>[] irisToClasses(final Set<IR
}

// A statically initialized Map of IRIs to HQDM classes.
private static final Map<IRI, java.lang.Class<? extends Thing>> iriToClassMap = new HashMap<>(250);
private static final Map<IRI, java.lang.Class<? extends Thing>> iriToClassMap = new HashMap<>(400);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this increased to 400? Was it to create headroom in case additional records are to be added? If so, this still seems to be an arbitrary number.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's the starting capacity of the HashMap to prevent costly reorganisation of the data structure during its initial population. 250 was fine until we added the ability to extend HQDM, because extension modules can add to this map, and yes 400 is an arbitrary number which it probably has to be since we won't know if any extensions will be added or how many, so eventually the HashMap will have to reorganise itself anyway when there are many extension classes.

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 HqdmObjectFactory
3 participants