record-builder icon indicating copy to clipboard operation
record-builder copied to clipboard

Suggestion: protected constructor on builder

Open jedvardsson opened this issue 3 years ago • 11 comments

Hi, I appreciate all you work on this project. Thank you.

What do you think about generating abstract builder classes (similar to AutoValue). This would allow for greater flexibility to add convenience methods and implement other interfaces? Like so,

@RecordBuilder
public record Example(String name) {

    public static CatalogBuilder newBuilder() {
        return new Builder();
    }
    
    public static final class Builder extends ExampleBuilder implement OtherStuff {
    }
}

jedvardsson avatar Jun 27 '22 08:06 jedvardsson

The only thing needed for this is that the generated builder's constructor isn't private right?

Randgalt avatar Jun 27 '22 09:06 Randgalt

Yes, I suppose so. However, the newBuilder-method on the builder class will not work as expected. Therefore, to keep compatibility it might be better to make the generated builder abstract and add an abstract newBuilder-method, or leave it out entirely. Maybe new flag on the annotation abstractBuilder=true?

jedvardsson avatar Jun 27 '22 09:06 jedvardsson

Yeah - I'm open to that if it's hidden behind an option as you suggest. My only concern is that it doesn't lock the library into anything for the future. Right now, for example, there are two private constructors. Other versions of the generated builder with other options generate constructors that are different. So, I wonder how tricky this will be to implement.

Randgalt avatar Jun 27 '22 09:06 Randgalt

Maybe this isn't a good idea after all. Careless subclassing might messup equals and hashcode.

jedvardsson avatar Jun 27 '22 09:06 jedvardsson

Agree - it might be a compromise to allow injecting super-interfaces that have default methods in it or something.

Randgalt avatar Jun 27 '22 09:06 Randgalt

Yes, that is probably better.

jedvardsson avatar Jun 27 '22 09:06 jedvardsson

I'm using the builders like this

@MyRecordBuilder
public record Costs(
    @Nullable BigDecimal shipping,
    @Nullable BigDecimal tax
)
{

    public CostsBuilder toBuilder()
    {
        return CostsBuilder.builder(this);
    }

    public static CostsBuilder builder()
    {
        return CostsBuilder.builder();
    }

}

IMHO it's not that much more work to add these two methods.

fprochazka avatar Jul 19 '22 13:07 fprochazka

@fprochazka, we are not talking about the “factory” methods which indeed are public already, but the the private constructor which prevents subclassing. However, it might not be a good idea after allow that.

Anyway, thanks for trying to help out. 👍

jedvardsson avatar Jul 22 '22 17:07 jedvardsson

Yeah, I got that, I was just wondering why you'd want to subclass it?

fprochazka avatar Jul 22 '22 20:07 fprochazka

Those who want to leverage the Builder as a parent class for a Java Bean. See #128.

toolforger avatar Sep 15 '22 19:09 toolforger

To implement interfaces and add additional methods.

22 juli 2022 kl. 22:43 skrev Filip Procházka @.***>:

 Yeah, I got that, I was just wondering why you'd want to subclass it?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

jedvardsson avatar Oct 11 '22 07:10 jedvardsson

any plans to add support like this? i would love a good way to customize the builder. i've often wanted to add convenience methods to the builder. for example, i often use records for collections of options. there may be a common collection of options and i'd love to be able to add a convenience method like:

public Builder commonOptions() {
  option1();
  option2();
  option3();
  return this;
}

jahlbornG avatar Aug 24 '23 13:08 jahlbornG

At this point I'd like to limit any new customizations. This library ran into a lot of problems with some of the recently added customizations.

Randgalt avatar Sep 20 '23 06:09 Randgalt

The latest release has an option to make the ctors public

Randgalt avatar Dec 21 '23 17:12 Randgalt