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

Use TreeMap in Response #1198

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

Conversation

Isira-Seneviratne
Copy link
Member


Store the response headers in a TreeMap. This allows the header value retrieval to be optimised as TreeMap orders its keys using a comparator.

@TobiGr
Copy link
Member

TobiGr commented Jul 20, 2024

Good catch. iterating through the map elements does not make any sense. Using the default get / getOrDefault method is the proper way to go 👍

However, I do not understand why we should specifically use a TreeMap. It guarantees a look-up cost of log(n). Same for put operations. However, the usually used HashMap has O(1) if the bucket size is good and not too many items are in the Map. There shouldn't be hundreds of elements in the map, but rather 25-50. So I think using passed map is more efficient than creating a new Map which needs to process all elements first. This is unnecessary pre-processing and comes with additional computation time and requires additional space.

@Isira-Seneviratne
Copy link
Member Author

Isira-Seneviratne commented Jul 20, 2024

I've revised the code a bit, to reuse the map if it is already a SortedMap. That way, no extra objects need to be created in that scenario, such as with OkHttp's header map implementation.

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.

2 participants