Special matching for Optional types

[umputun]

This is the example of annotated constructor with "partial" match on last parameter (String instead of Optional<String> )

   public final String name;
   public final String someTestFld;
   public final Optional<String> someOtherFld;

    @GeneratePojoBuilder(withCopyMethod = true)
    public User(String name, String someTestFld, String someOtherFld) {
        this.name = Objects.requireNonNull(name);
        this.someTestFld = Objects.requireNonNull(someTestFld);
        this.someOtherFld = Optional.ofNullable(someOtherFld);
    }

Currently PB handles such code almost right. The only issue I can see is a lack of this filed in copy() method.

  public UserBuilder copy(User pojo) {
    withSomeTestFld(pojo.someTestFld);
    withName(pojo.name);
    return self;
  }

This special support of Optional will be great to have because such use case seems to be very practical one for non–mandatory fields.

based on discussion in #88

[mkarneim]

I am sorry that I have not commented on this enhancement request for such a long time. I haven't had time to think it thru in depth. I'll see if I find some time the next week.

[umputun]

Hey. Have you had a chance to think it thru? I'm dealing with a lot of such value objects and they usually have some mandatory and some optional fields. Supporting Optional semantics (in fields declaration) with convenience of builder will be very, very welcome addition.

[mkarneim]

Ah, sorry, still short of time...

I spent some hours thinking on this and still have no working solution. I also tried some quick hacks but they didn't work out.

But I can tell some of my insights.

So, let's start with the analysis: Why does the copy method NOT contain a statement like withSomeOtherFld(pojo.someOtherFld.get());?

The reason is that PB does not know that withSomeOtherFld and pojo.someOtherFld do logically point to the same property. Actually, for PB your class has 2 distinct properties with the name "someOtherFld". This has two reasons: 1) PB identifies a property by the uniqe combination of the property's name and type. 2) PB searches three places for properties: field declarations, setter methods, and constructor parameters.

Hence there are two properties:

• Optional<String> someOtherFld (declared as a field)
• String someOtherFld (declared as a constructor parameter)

When PM generates the copy method it loops over all "readable" pojo properties for which a matching "with" method exists. In this case the first property Optional<String> someOtherFld is readable, but has no matching "with"-Method (in detail: there is no withSomeOtherFld(Optional<String>) in the builder because the pojo's field is declared final). And the second property String someOtherFld has a "with"-method, but is not readable (because, there is no field declaration String someOtherFld nor a corresponding getter method.)

So, what could we do now? To solve this we could soften the property matching logic so that PB treats Optional types (yeah more than one, since we have at least Java8 Optional and Guava's) like their generic type parameter. What I like about this approach is that it possibly could be used also as a solution for supporing Java's primitive wrapper classes (used for Autoboxing). For me this feels the right way to do it but this definitely requires a greater modification of the PB code - probably with some unknown complications. Maybe there is a shortcut to this issue, but right now I don't see it.

So, how to proceed? My problem is that I currently do not find the time to dig deeper into this issue nor to do some prototyping. PB is just some spare time / leasure time project. Also I am not convinced if this feature has a high priority since I think that using Optional objects as pojo properties is in fact an anti pattern (see this discussion), but of course this is no reason if PB users use Optionals like this anyway :-)

However, I understand that this issue is important for you, and I definitely want to support you in that. Perhaps you (or anybody else who's into it) have already an idea how to solve it and have some time to contribute code to this project? I will be happy to include a solution to this issue into the code base!

Apart from that I will work again on this issue when I find time for it. I keep this issue open.

[mkarneim]

FYI, here is a feature branch for this: https://github.com/mkarneim/pojobuilder/tree/optional-support

It just implements a failing test.

[drekbour]

Not sure if #134 closes this?