[core] Builder pattern for properties#669
Conversation
| */ | ||
| public abstract class PropertyDescriptorBuilder<E, T extends PropertyDescriptorBuilder<E, T>> { | ||
|
|
||
| protected String name; |
There was a problem hiding this comment.
considering the name is required (there is no way to use a property without a name), I'd suggest having a constructor to set it directly instead of having to call name() each time, which will:
- reduce errors
- remove some boilerplate
Maybe go further with something such as IntegerProperty.named("myProp").desc(....)....build()
There was a problem hiding this comment.
I'm guessing, it replaces the boilerplate by some other boilerplate ^^ But I agree with the rationale
| * @return The same builder | ||
| */ | ||
| @SuppressWarnings("unchecked") | ||
| public T defalt(List<V> val) { |
There was a problem hiding this comment.
defaultValue? I rather have a longer name than a typo :P
| * @return The same builder | ||
| */ | ||
| @SuppressWarnings("unchecked") | ||
| public T defalt(E val) { |
There was a problem hiding this comment.
defaultValue? I rather have a longer name than a typo :P
| * @return The same builder | ||
| */ | ||
| @SuppressWarnings("unchecked") | ||
| public T min(V val) { |
There was a problem hiding this comment.
considering NumericPropertyModule requires both, lowerLimit and upperLimit to be set, we may want to add a T range(V min, V max) method to reduce boilerplate a little further.
Checks item 4 of #504
I did my best but it feels like so much boilerplate lol. With that builder thing going on, what about deprecating the constructors? Maybe not even to remove them, but to be able to change them when we need it, and to warn users there's a better way
Cheers!