mkarneim / pojobuilder

A Java Code Generator for Pojo Builders
Other
334 stars 44 forks source link

Use initialized fields when creating object #107

Closed jukkasi closed 8 years ago

jukkasi commented 8 years ago

We have multiple objects like

class Foo {
    private List<A> as = new ArrayList<>();
    private List<B> bs = new ArrayList<>();
    private Bar bar = new Bar();

    @GeneratePojoBuilder
    public Foo(List<A> as, List<B> bs, Bar bar) {
        this.as = as;
        this.bs = bs;
        this.bar = bar;
    }
}

Especially for collections, we don't want to handle null objects.

When creating such object with PojoBuilder the problem is that field initializations are not taken into account since builder uses constructor directly with all fields set by builder or null. I can work around this by using copy feature like new FooBuilder().copy(new Foo()) but it's not very convenient to say this everywhere.

Is it possible that builder actually would use initial values for fields?

mkarneim commented 8 years ago

Well, at our place we use a Builder Factory to define default values for builders. Another option is to annotate a factory method instead of the pojo's constructor. There you can handle any null values and silently replace them with empty lists. But in any case, if your pojo does not allow null values, I suggest your pojo's constructor should throw a NPE if null values are passed into it.

jukkasi commented 8 years ago

I understand your explanation and already looked at builder factory / factory method.

However the problem with this approach is that I would need to define my defaults (or actually in my case "empty" state of the pojo) twice, in field initializations and in factory method. Also, creating a factory method for every pojo I need is extra work especially when multiple builders are needed.

Actually its not about allowing (or disallowing) null values but using the initial state of the pojo as starting point for building.

mkarneim commented 8 years ago

Ok, can you please elaborate this? How is your example class using the default values? Can you please show the constructor's implementation?

Or can you show me a more real-world example?

jukkasi commented 8 years ago

The example in my original post is my actual use case, I've updated the constructor implementation.

If I create my pojo by Foo pojo = new Foo(), the initial "empty" state includes empty (not null) lists and initialized bar. This is what I want since I want to assume that there are no null collections in my pojos when using them.

Now, if I create my pojo by Foo pojo = new FooBuilder().build() the initial state is not what I want since the field initializations are not included in FooBuilder.

I understand that FooBuilder uses the constructor for building the pojo and how it works, and I'm not sure at this point how the functionality I'm hoping for could be achieved.

Only way I could think is that FooBuilder would create new Foo() and then set desired fields by reflection if there is no setters available.

mkarneim commented 8 years ago

Hmm, I still feel that the example is not complete...

Why does Foo define any values for as, bs, and bar? I mean, these values are effectily not used, since they are replaced as soon as the constructor is finished, aren't they?

But perhaps you meant that Foo has another, parameter-less constructor?

class Foo {
    private List<A> as = new ArrayList<>();
    private List<B> bs = new ArrayList<>();
    private Bar bar = new Bar();

    @GeneratePojoBuilder
    public Foo(List<A> as, List<B> bs, Bar bar) {
        this.as = as;
        this.bs = bs;
        this.bar = bar;
    }

    public Foo() {
    }

}

This means that anyone can call new Foo() - and the default values for all 3 properties are still in place.

But anyway, this doesn't solve the problem either. Since PB always uses the parameterized constructor (because you annotated it), all default values are overwritten when the generated builder is used.

However, this is not PB specific. How would you manually create a new Foo with some specified 'bar' but keeping the default values for 'as' and 'bs'? You can't. You either can specifiy all 3 parameters or none of them.

new Foo(new Bar(), ?, ?);

To solve this I have two different solutions:

1) Use Setter Methods

For example, you could remove the parameterized constructor and instead, provide some setter (and getter) methods.

class Foo {
    private List<A> as = new ArrayList<>();
    private List<B> bs = new ArrayList<>();
    private Bar bar = new Bar();

    @GeneratePojoBuilder
    public Foo() {
    }

    public void setBar(Bar bar) {
        this.bar = bar;
    }

    public void setAs(List<A> as) {
        this.as = as;
    }

    public void setBs(List<A> bs) {
        this.bs = bs;
    }
}

Now the default values are only replaced when somebody calls one of the setter methods.

And fortunately PB handles this correctly:

Foo foo = new FooBuilder().withBar( new Bar()).build();
assert foo.as != null;
assert foo.bs != null;

But perhaps you can't add setter methods to Foo, because Foo must be immutable?

Well, then you could do some logic inside the constructor:

2) Make the Costructor Parameters Nullable

public static class Foo {
    private List<A> as = new ArrayList<>();
    private List<B> bs = new ArrayList<>();
    private Bar bar = new Bar();

    @GeneratePojoBuilder
    public Foo(@Nullable List<A> as, @Nullable List<B> bs, @Nullable Bar bar) {
      this.as = as != null ? as : this.as;
      this.bs = bs != null ? bs : this.bs;
      this.bar = bar != null ? bar : this.bar;
    }
  }

Here you define a special contract: passing Null values as parameters means that Foo will ignore them.

Does this help?

jukkasi commented 8 years ago

You are absolutely correct, it was my mistake not to include default constructor Foo() in my example which really is the case.

Thank you for your detailed response, I will look at your suggestion 2) since I feel more comfortable without setters

mkarneim commented 8 years ago

You are welcome!