With-methods for "withBuilderProperties = true" are generated twice when supporting both primitive and non primitive setter methods

[Adrodoc]

With-methods for "withBuilderProperties = true" are generated twice when supporting both primitive and non primitive setter methods. This will lead to a builder with compiler errors.
Example:

@GeneratePojoBuilder(withBuilderInterface = Builder.class, withBuilderProperties = true)
public class Pojo {
  private int value;

  public void setValue(int value) {
    this.value = value;
  }

  public void setValue(Integer value) {
    this.value = value;
  }
}

This is also an issure when using different types in setter methods and an annotated constructor:

public class Pojo {
  private int value;

  @GeneratePojoBuilder(withBuilderInterface = Builder.class, withBuilderProperties = true)
  public Pojo(Integer value) {
    if (value == null) {
      this.value = 12345;
    } else {
      this.value = value;
    }
  }

  public void setValue(int value) {
    this.value = value;
  }
}

In both cases the method public PojoBuilder withValue(Builder<Integer> builder) is generated twice.
I'd suggest to not generate the with method for the primitive type, but only for the Wrapper type. That way the given builder is allowed to build null values.

[mkarneim]

I checked this out today. Technically your example is just a special case of having several properties with the same name but with different types. Here is an example:

@GeneratePojoBuilder(withBuilderInterface = Builder.class, withBuilderProperties = true)
public class Pojo {
  private int value;

  public void setValue(int value) {
    this.value = value;
  }

  public void setValue(String value) {
    this.value = Integer.parseInt(value);
  }
}

These two setter-methods are interpreted as two distinct properties with the same name but with different types (int and String). Similar to your example this will cause a compiler error

Erasure of method withValue(Builder<String>) is the same as another method in type PojoBuilder

since PB generates a builder-based with-method for both setter-methods.

So the basic problem is: how to handle builder-based with-methods if there are several properties having the same name?

I my opinion this can not be solved neatly except of just skipping the generation of one or the other.
But how to decide which one to skip? Of course, in your example there is some natural way of deciding that, but this does not help with the basic problem.

This, and the fact that even for your case the implementation of the solution is kind of tricky, makes me feel that it is not worth the effort. Since there is a work-around for dealing with this kind of errors, I suggest that you go with it.

Just exclude the second property completely:

@GeneratePojoBuilder(withBuilderInterface = Builder.class, withBuilderProperties = true, 
  excludeProperties = {"value:int"})
public class Pojo {
  private int value;

  public void setValue(int value) {
    this.value = value;
  }

  public void setValue(Integer value) {
    this.value = value;
  }
}

Comments?

[mkarneim]

@Adrodoc55 what do you think? Can we close this issue?

[Adrodoc]

@mkarneim it might be a good idea to post a compiler error on the Annotation of the Pojo wich suggests the workaround. That way it is easier for developers and it does not look like a bug of pojobuilder. To make it easier to exclude certain properties I opened #118.

[mkarneim]

Alright, I label this as a feature request.

[Adrodoc]

Because I run into this special case again and again (in my company) i think it might be worth handling BoxClasses as a special case here.

[mkarneim]

Can you elaborate this?

[Adrodoc]

As you pointed out my original issue of having a primitive and a non primitive setter was just a special case of having two different "properties" with the same name. If the types don't correlate there is no natural way of deciding wich builder method to generate, but in case of box classes there is. The reason why this comes up a lot is because we are using static factory methods where we want to differentiate between a user specified default value and no specification at all. For example if I have a class like this:

public class Interval {
  public int min;
  public int max;
}

This class has a database constraint so that min must be smaller than max. When the User only specified one of the two properties:

new IntervalBuilder().withMin(5);

Then I can calculate the missing value in my static factory method. I can only so that if I know that the user did not specify both values. If my static factory method looks like this:

public static Interval createInterval(int min, int max) { ... }

Then I can't know whether the use specified max to be 0, or did not specify max at all. I want to calculate a correct max value only if the user did NOT specify any value for max.
For this reason we often use BoxClasses in factory methods:

public static Interval createInterval(Integer min, Integer max) { ... }

That way I can treat null as "not specified" (for nullable properties this is a bit more complicated so I won't go into it here). But now I have to exclude the properties min:int and max:int, because they are treated as different properties than min:Integer and max:Integer. I have to do this for almost every static factory method, so I thought it might be worth to treat BoxClasses als a special case and exclude the primitive property automatically in case of a conflict between a primitive and it's BoxClass. Or in case of a constructor/factory method exclude the non required setter/field. Of course if the BoxClass is already excluded by the user, there is no need to exclude the primitive property. If neither Optional nor a Builder interface is used there is also noch reason to do the automatic exclude.
Do you think the same way, or so you think this is a very special case that is not worth special treatment?

[mkarneim]

Thank you, I understand now.
You are right, in this case it is clear which of both properties should be excluded.
I'd like to include that into PB. Do you want to contribute the code?

[Adrodoc]

@mkarneim Not at the moment