Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid creating extra constructor on builders for records #71

Closed
mdewilde opened this issue Aug 8, 2024 · 1 comment
Closed

Avoid creating extra constructor on builders for records #71

mdewilde opened this issue Aug 8, 2024 · 1 comment

Comments

@mdewilde
Copy link

mdewilde commented Aug 8, 2024

You added the ability to create builders for records (issue 69) after I requested it last year. Thanks for that, I use it often.

I'd like to request a tweak to the logic.

When generating a builder for a record, the plugin should not create a private constructor in the record that takes a Builder instance.

That is because a record by its very nature always has a public full-argument constructor which can be called from the Builder.

As an example, this is what the plugin currently does:

public record Request(Object argument) {

	private Request(Builder builder) {
		this(builder.argument);
	}

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

	public static final class Builder {
		private Object argument;

		private Builder() {}

		public Builder argument(Object argument) {
			this.argument = argument;
			return this;
		}

		public Request build() {
			return new Request(this);
		}
	}

}

This is what I believe would be a better alternative:

public record Request(Object argument) {

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

	public static final class Builder {
		private Object argument;

		private Builder() {}

		public Builder argument(Object argument) {
			this.argument = argument;
			return this;
		}

		public Request build() {
			return new Request(argument);
		}
	}

}

Less code and a more strict single canonical constructor for record instances.

helospark added a commit that referenced this issue Aug 20, 2024
helospark added a commit that referenced this issue Aug 20, 2024
@helospark
Copy link
Owner

Hi @mdewilde !
I released this change in 0.0.29.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants