Hi,
>So I'm -0.5 on factory methods for all components using lambda's for
>expressions. Especially now. We can always add them later in a 8.y
>update (additions are not breaking semver). Not including them now
>will prevent folks from going down a path that's yet not very well
>understood.
I share your concerns, IMHO we could start without these.
Note that AjaxFormSubmitBehavior#onSubmit(Consumer, Consumer) is another
offender, maybe here your suggested #onSubmitOnError() could be used.
Regards
Sven
Am 12.11.2016 um 16:06 schrieb Martijn Dashorst:
Consider the following Wicket 7.x code:
form.add(new AjaxSubmitLink("opslaan")
{
private static final long serialVersionUID = 1L;
@Override
protected void onSubmit(AjaxRequestTarget target)
{
modalWindow.getEldDossierAanvraagOverzichtPage()
.scheduleEldDossierImporterenJob(createDossierAanvragen());
modalWindow.close(target);
}
@Override
protected void onError(AjaxRequestTarget target)
{
refreshFeedback(target);
}
});
Now I tried rewriting this to the factory method of ASL:
form.add(AjaxSubmitLink.onSubmit("opslaan", (l, t) -> {
modalWindow.getEldDossierAanvraagOverzichtPage()
.scheduleEldDossierImporterenJob(createDossierAanvragen());
modalWindow.close(t);
}, (l, t) -> {
refreshFeedback(t);
}));
If you compare this code with the original, it is more concise, but
fails the clarity test.
The naming of the factory method in combination with the lambda's
hides the functionality of the two lambda's. It is unclear which
lambda performs which role (the parameters have the right names, but
don't expose their name to the outside world).
Renaming the factory method to 'onSubmitOnError' improves it slightly:
form.add(AjaxSubmitLink.onSubmitOnError("opslaan", (l, t) -> {
modalWindow.getEldDossierAanvraagOverzichtPage()
.scheduleEldDossierImporterenJob(createDossierAanvragen());
modalWindow.close(t);
}, (l, t) -> {
refreshFeedback(t);
}));
But perhaps we are going the wrong way when we propose such an API?
Should this perhaps rather be a Builder for AjaxSubmitLink?
form.add(AjaxSubmitLink.id("opslaan")
.onSubmit( (l, t) -> {
modalWindow.getEldDossierAanvraagOverzichtPage()
.scheduleEldDossierImporterenJob(createDossierAanvragen());
modalWindow.close(t);
}).onError((l, t) -> {
refreshFeedback(t);
}).build());
This is workable, but crafting the Builder is quite the undertaking,
especially assembling the final component. I am thinking that perhaps
the best way to do that is not to try to subclass each component, but
rather start composing components using behavior(s).
That said, I think performing Lambda-injection into all components and
facets of Wicket might not be the best for our API, without really
considering the ramifications for client code.
The builder approach looks ok-ish, but is only a few lines shorter
than a proper anonymous inner class. We save a couple of "@Override"
lines and a serialVersionUID line, and adds a lot of state to the
component.
So I'm -0.5 on factory methods for all components using lambda's for
expressions. Especially now. We can always add them later in a 8.y
update (additions are not breaking semver). Not including them now
will prevent folks from going down a path that's yet not very well
understood.
Just my thoughts...
Martijn
