Randgalt / record-builder

Record builder generator for Java records
Apache License 2.0
756 stars 55 forks source link

Suggestion: protected constructor on builder #121

Closed jedvardsson closed 11 months ago

jedvardsson commented 2 years ago

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 {
    }
}
Randgalt commented 2 years ago

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

jedvardsson commented 2 years ago

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?

Randgalt commented 2 years ago

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.

jedvardsson commented 2 years ago

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

Randgalt commented 2 years ago

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

jedvardsson commented 2 years ago

Yes, that is probably better.

fprochazka commented 2 years ago

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.

jedvardsson commented 2 years ago

@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. 👍

fprochazka commented 2 years ago

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

toolforger commented 2 years ago

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

jedvardsson commented 2 years ago

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.

jahlbornG commented 1 year ago

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;
}
Randgalt commented 1 year ago

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 commented 11 months ago

The latest release has an option to make the ctors public