Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

More flexible MapSplitter API #3491

Open
tkowalcz opened this issue Jun 2, 2019 · 5 comments
Open

More flexible MapSplitter API #3491

tkowalcz opened this issue Jun 2, 2019 · 5 comments
Assignees
Labels

Comments

@tkowalcz
Copy link

tkowalcz commented Jun 2, 2019

I name the issue vaguely as I don't want to force any particular change. I will describe my use case and then one possible solution.

Currently MapSplitter has only one method:

Map<String, String> split(CharSequence sequence)

and returns immutable collection.

My use case is that I would like to change the resulting collection, e.g. by appending some default values for missing entries. I also want to reduce GC pressure and not create unnecessary collections along the way.

One solution that comes to mind is to provide the map to be used for storing the result:

void splitInto(CharSequence sequence, Map<String, String> target)

Instead of map it can be a factory argument:

void splitInto(CharSequence sequence, Supplier<Map<String, String>> mapSupplier)

Additionally both approaches would allow for hash map pooling if required. I can work on a PR if this API addition makes sense.

@nick-someone
Copy link
Member

nick-someone commented Jun 3, 2019

Can you provide a little more context about the problem you're looking to solve with a mutable result parameter?

I see that you want to modify the result by adding default values and want to avoid adding garbage by doing things like copying the result of split into your own mutable HashMap, but, for example, are you performing this map split on a performance-sensitive codepath that happens frequently?

One thing you can consider in the meantime is to unpack the Splitter and manually split the entry set:

Splitter outer = Splitter.on(';');
Spliiter entrySplitter = Splitter.on("=");
outer.split(stringValue).forEach(entry -> {
  List<String> values = entrySplitter.splitToList(entry);
  // Doesn't check for well-formedness of entry, uniqueness of entries within map, etc.
  map.put(values.get(0), values.get(1));
});

@cpovirk
Copy link
Member

cpovirk commented Jun 3, 2019

A few months ago, we decided to add Splitter.spiltToStream, which might also help here. However, it doesn't look like we ever implemented it.

Nor did we explicitly discuss a MapSplitter equivalent. Maybe we could decide to add one... or maybe we'll block it on deciding whether we want some kind of MapStream/BiStream type....

@orionll
Copy link

orionll commented Feb 17, 2020

Also, it would be great to split into ImmutableMap.

@dimo414
Copy link
Contributor

dimo414 commented Feb 18, 2020

FWIW we did get Splitter.splitToStream() in for 28.2, but I don't believe map-splitting has come up yet. @kevinb9n ?

@kluever
Copy link
Member

kluever commented Feb 18, 2020

Also, it would be great to split into ImmutableMap.

Unfortunately that's not possible because it would create a circular dependency between base and collect.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants
@tkowalcz @nick-someone @orionll @cpovirk @dimo414 @kluever and others