Use chaining for all Builder parameters#508
Conversation
There was a problem hiding this comment.
This is defintely more consistent
There was a problem hiding this comment.
Could you keep it as it were? Thx
There was a problem hiding this comment.
Sure, I'll bring this back in my amended commit.
There was a problem hiding this comment.
any specific reason to delete this?
There was a problem hiding this comment.
My interpretation of the pattern was that ExecutionStrategyParameters had access to the constructors of Builder, while outside classes do not. Given that, ExecutionStrategyParameters#transform(Consumer) could call the constructor directly instead of via an additional layer of indirection (the static accessor of the constructor). There were no external callers of ExecutionStrategyParameters#newParameters(ExecutionStrategyParameters), so it was safe to delete.
What's the reason for using this method?
There was a problem hiding this comment.
yes, you are right: it would be nicer to just call the copy constructor directly and remove this method.
tklovett
force-pushed
the
builder-consistency
branch
2 times, most recently
from
7e7aa5e to
63a333f
Compare
There was a problem hiding this comment.
In general I like this PR ...thank you!
See my comments about final for a change I would like to see.
It is a bit hard to review because of the reordering of code. As a general suggestion: I think it would be better to split up changes and reordering in the future.
Thanks again
There was a problem hiding this comment.
could you remove the final initialiser for the arguments? also for the other methods? thanks
tklovett
force-pushed
the
builder-consistency
branch
from
63a333f to
f66494b
Compare
|
I tried to fix this up as a maintainer but I cant push upstream to the originating git My git fu is not super strong I have created #524 which is this with the conflcts resolved |
4 participants
Do you want this change? In summary, it maintains the
Foo.Builder newFoo()pattern but moves any parameters passed tonewFoomethods to chaining methods.