Add primitive support to the map(...) function in generated builders.

[masterav10]

The map(...) function provided by Builders always uses UnaryOperator, even when the field is a primitive.

For the given interface:

@FreeBuilder
interface Hello {
    long value();
}

I would expect the following function in my Builder:

Builder mapValue(LongUnaryOperator mapper);

but instead I get:

Builder mapValue(UnaryOperator<Long> mapper);

This incurs 2 autoboxing operations, which may be expensive depending on the frequency of use.

I realize that in some cases the appropriate function interface is not available (boolean, byte, short), but the most commonly used and expensive primitives have these interfaces. For these datatypes, the cost likely justifies the change. The impact on existing clients will likely be minimal since I imagine 99% of the implementations use the value directly in the lambda.

[alicederyn]

This is an excellent point which I wish we'd spotted when implementing map. Unfortunately this would now be a binary-incompatible change for clients, and I don't have a great idea for how to do those.

[masterav10]

If binary compatability is a major concern, we could provide an additional method specifically for those primitives, such as

Builder mapValueAsAPrimitive(LongUnaryOperator mapper);
Builder mapValueEfficiently(LongUnaryOperator mapper);

Assuming this route is chosen, it makes sense to put a note in the documentation for the original method to favor using this implementation if the map function is called frequently.

[alicederyn]

Or mapUnboxedValue

[chrisrhut]

Came here by searching issues for "primitive" because I'm running into this concern. One possible suggestion (and please let me know if I should submit this as a separate issue).. It seems that an area where this is always likely to be an issue is when accumulating a value (perhaps @masterav10 can chime in if this was their use case, or maybe I'm totally off-base 😄). E.g.:

final List<Stats> stats; // .. from somewhere...
final Stats.Builder total = new Stats.Builder();

stats.forEach(stat -> {
  total.mapFoo(v -> v + stat.getFoo());
  total.mapBar(v -> v + stat.getBar());
});

I know that API explosion is a risk, but I wonder if an increment() method for the numeric primitives would be appropriate? So the above turns into:

stats.forEach(stat -> {
  total.incrementFoo(stat.getFoo());
  total.incrementBar(stat.getBar());
});

Thanks again for your time on this library!

[masterav10]

This is the case. I use FreeBuilder for my data objects, one of which represents metadata for imagery using a long index. It is updated at roughly 30 FPS.

Working on an implementation in copious spare time.

[alicederyn]

I'm willing to 2.0 FreeBuilder to fix this properly.

[alicederyn]

We just need a way to restore the old API for anyone not wanting to break back-compat. Detecting an override with the old signature is the way we generally do this.

[masterav10]

I assume the override would be in the builder?

Would you rather approach the problem this way versus adding an extra method? I have no preference.

[alicederyn]

In the builder, yes.

Definitely better to fix the original mistake rather than work around it.

[alicederyn]

@masterav10 Are you still working on this?

[alicederyn]

You can now pick up this fix in FreeBuilder v2.

[chrisrhut]

Thank you for following up and congrats on the 2.x release!