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

Take parameters in .builder() #237

Closed
gabbard opened this issue Jan 14, 2016 · 2 comments
Closed

Take parameters in .builder() #237

gabbard opened this issue Jan 14, 2016 · 2 comments

Comments

@gabbard
Copy link

gabbard commented Jan 14, 2016

It would be nice if same @Parameter fields used for of static factory methods were also be required as arguments of .builder(). This prevents the case where the user forgets a required argument when using the builder but doesn't find out about it until runtime. Example:

@Value.Immutable(buildersRequireParams=true)
abstract class Person {
   @Parameter
   public abstract String name();
   public abstract Optional<String> jobTitle();
 }

final MyClass fred = ImmutableMyClass.builder("Fred").build();
final MyClass sally = ImmutableMyClass.builder("Fred").jobTitle("CEO").build();
// but not ImmutableMyClass.builder().jobTitle("CEO")

This would of course need to be turned on by a style.

@elucash
Copy link
Member

elucash commented Jan 15, 2016

Thank you for the feature request! For some "gut feeling" reason, I would not conflate here constructor parameters and builder parameters, mostly because how it can be combined in a different way. Yes, we can introduce another one style attribute for this as you proposed, but we have a lot better chances to reuse existing functionality. We have builder module with annotations applicable to special static factory builders. Annotation @Builder.Parameter (and also @Builder.Switch just to mention) can only be used on parameters of a static method. I'll played a bit with code so it will work now for attributes and I guess annotation name reveals the intent well. Oh yeah, it would be funny to see an attribute with two annotations @Value.Parameter @Builder.Parameter if used together. After experimenting with the code I also found out that we cannot remove parameter-less builder() method or make it private because quite few other generators depends on this convention and changing them would be too painful, so builder() method (and separate initializers for attributes turned into parameters) were made package-private.
You can try latest snapshot/build from sources to check this out. Remember to add org.immutables:builder module with annotations as a compile dependency.

@elucash elucash added this to the 2.1.8 milestone Jan 15, 2016
@elucash
Copy link
Member

elucash commented Jan 16, 2016

Released as is in 2.1.8

@elucash elucash closed this as completed Jan 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants