Make builder constructor public

[toolforger]

The purpose is to make record-builder fit for a slightly off-label use case:
Generate the getter/setter/equals/hashcode/toString boilerplate for our otherwise standard Java Bean classes.
We don't oppose records, actually, it's just that some libraries don't fully support using them yet.

We would like to be able to make a bean class Foo like this:

public class Bean extends FooBaseRecordBuilder {
    // We need a parameterless constructor for a Java Bean.
    // If the parent class has it but makes it private, we can't even create it here.

    // Any overrides for getters/setters go here.
    // (If there are no overrides, maybe we can configure record-builder to create Foo directly.
    // We did not pursue that venue since our team is okay with having an empty Foo class.)
}

where FooBaseRecordBuilder is generated from a FooBase interface:

@RecordInterface
// Actually this is in a @RecordBuilder.Template :-)
@RecordBuilder.Options(booleanPrefix = "is", getterPrefix = "get", setterPrefix = "set")
public interface FooBase {

    boolean isCustomizable();

    @JsonView(Patchable.class) // Probably need a @RecordBuilder.Options entry to have this on the actual Java Bean
    boolean isEnabled();
}

Aside notes:

• We do not need the generated FooRecord class.
• In the FooRecordBuilder class, we need only the default constructor, the fields, getters/setters, and equals/hashcode/toString.
• The stream is a very, very appreciated addition that will help us with some other stuff. We interact with MongoDB and Jackson, and being able to iterate over whatever fields are in an entity object is a perfect match for these. (It would be nice to have a stream that returns the getters and setters, but we have to encounter that use case yet.)

[Randgalt]

This shouldn't be in the project - please remove

[toolforger]

Actually that's a manually written comment meaning "the directory below is generated by Maven".

[Randgalt]

Let's let this sit for a bit to see if others have feedback. Regardless, this should have an option to enable it. It should not be the default.

[toolforger]

Let's let this sit for a bit to see if others have feedback.

Agreed.

Regardless, this should have an option to enable it. It should not be the default.

I was considering it.
Thought it's adding to the burden of things one has to configure, so I looked whether it's necessary.

Things that speak against making it configurable:

• No existing code will be affected.
• There's no need to prevent subclassing, as the Builder is mutable (by design), and I have yet to see another reason than immutability to prevent subclassing.

Things that speak for making it configurable:

• If somebody comes up with a reason for making it un-subclassable by default, changing the default would be incompatible.

My personal take is that we should see if somebody comes up with a guarantee that an unsubclassable builder can give that a subclassable does not.
That said, if you think a configuration option is the way to go, I prefer a fast decision even more and will happy add the option.

[Randgalt]

That said, if you think a configuration option is the way to go, I prefer a fast decision even more and will happy add the option.

Yes, please.

[toolforger]

I see that to allow a record builder to be both subclassable and fluent, it needs an additional type parameter for the setters' return type.

This has ramifications for type parameters on Withers, factory methods, and such, but I don't have a useful overview.
Should I just add the new type parameter to InternalRecordBuilderProcessor.typeVariables and "it will just work out", or do you expect problems?
What generated files should I look at to identify problems? (I suppose somewhere in record-builder-test/target/generated-sources, but which ones?)

[yurybubnov]

I got into a situation when I need a public constructor.
I'm working with AWS libraries which are written with Lombok-style generated builders in mind which have a public constructor in the builder and initialize builders via the said constructor. Pretty sure there are other libraries that are tailored towards Lombok-style builders.
It would be nice to have the option of making the default, no-arg constructor, public.

[Randgalt]

@yurybubnov when I get time I can enhance this to make it optional, etc. Or maybe you can do it if you can.

[KangoV]

This seems to be hanging around for a while. I'm moving from Immutables.io to this as I need to do some complex Jackson serialisation. In that framework i used to augment the builder with extra helpers, e.g:

public record Grouping(
    UUID id,
    String name,
    String description,
    String context,
    List<String> objectRefs )  {

    public static class Builder extends GroupingBuilder {
        public Builder id(String id) {
            this.id(UUID.fromString(id));
            return this;
        }
    }

    public static Builder build() {
        return new Builder();
    }

}

Just having both constructors on the builder as public or package would mean that this PR solves this use case. Another option for this would be to have a option to make the static builder() and builder(....) methods be package private. This would mean the only method would be the new one in the record itself.

This would mean the builder is "hidden". The immutables.io library does this.

[Randgalt]

tbh - I'm not sure what to do about this. A few bad commits got into RecordBuilder. Thankfully, they're behind options but they are there nonetheless. There's a new PR I haven't looked at. I'm not sure if it addresses this.

[KangoV]

I do think that this should be behind an option, builderConstructorVisibility maybe? The default being private. This would not break existing code.

[Randgalt]

@KangoV can you make a new PR with it behind an option?

[KangoV]

I'll have a go later and see how I get on.

[toolforger]

Anybody please feel free to take over and do as you wish.
The project I needed this for and me have parted ways, meaning my feedback wouldn't be very useful anyway.
(Also, not getting answers to my latest questions meant I wouldn't be triggered to continue work on this, so that's why there wasn't any progress. No offense taken though.)

[Randgalt]

(Also, not getting answers to my latest questions meant I wouldn't be triggered to continue work on this, so that's why there wasn't any progress. No offense taken though.)

Please accept my apologies for lack of response. The last year or so has left me little time for side projects. I'm trying now to back to this.

[toolforger]

No need to apologize, I know such things happen :-)
I just meant to give a heads-up that everybody please feel free to continue as is best for them, and an explanation that it's not anybody's fault.

[KangoV]

Should the default be public? I've had a look through the immutables.io project which I have been using and there is no option to make it private. Do you think an option is needed. I leaning towards not having one, unless someone shouts.

[toolforger]

It should be public if both apply:

• There is no expectation that the API changes as the library evolves (in this case, that it doesn't change unless the data that goes into it generation changes).
• It is useful for the users of the library, or enables library usage for new cases.

I believe both are true.

You may be reluctant to enable new use cases, since more varied use cases means more design constraints which means more design work and possibly more difficult design trade-offs.

I don't see big trouble ahead in this case, so I'd do it if it were my project.
On the other hand I don't see all use cases already in existence, and how narrowed-down the design space already is, so my instincts may be working with incomplete data.

[Randgalt]

I'd still like to be controlled by an option. This ship has sailed and I don't want to disrupt any existing use-cases.

[toolforger]

Not wanting to argue that the decision is already made... but making a constructor public isn't going to break any existing code, is it?

[Randgalt]

You'd be surprised. This is more of a gut reaction based on experience. I've made what I thought were innocuous changes in this and other libraries only to be surprised by the reaction of the community.

[Randgalt]

Replaced by #160