Skip to content

Conversation

@original-brownbear
Copy link
Contributor

Mostly the keys we read are strings, lets add an overload for that to save some code and maybe help the compiler make better decisions. Also readMapOfLists an be way simplified, no point in duplicating the map reading code here just to save one capturing lambda, there's not hot code that benefits from this.

Mostly the keys we read are strings, lets add an overload for that
to save some code and maybe help the compiler make better decisions.
Also readMapOfLists an be way simplified, no point in duplicating
the map reading code here just to save one capturing lambda, there's
not hot code that benefits from this.
@original-brownbear original-brownbear added >non-issue :Core/Infra/Core Core issues without another label v8.9.0 labels Jun 15, 2023
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Jun 15, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Copy link
Contributor

@arteam arteam left a comment

Choose a reason for hiding this comment

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

LGTM! That indeed looks much terser.

@original-brownbear
Copy link
Contributor Author

Thanks Artem!

@original-brownbear original-brownbear merged commit 795e07c into elastic:main Jun 15, 2023
@original-brownbear original-brownbear deleted the dry-up-read-map branch June 15, 2023 10:21
}
return map;
public <V> Map<String, List<V>> readMapOfLists(final Writeable.Reader<V> valueReader) throws IOException {
return readMap(i -> i.readList(valueReader));
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Core Core issues without another label >non-issue Team:Core/Infra Meta label for core/infra team v8.9.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants