On Sat, Nov 12, 2016 at 4:06 PM, Martijn Dashorst <
[email protected]> wrote:
> 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.
>
A better usage would be: form.add(AjaxSubmitLink.onSubmit("opslaan",
this::onOpslaanSubmit, this::onOpslaanError))
> 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...
>
Too bad Java doesn't have named parameters and default values.
The same method would be much more clear in Scala/Kotlin/Ceylon.
Do you suggest to leave #onSubmit(java.lang.String,
SerializableBiConsumer<AjaxSubmitLink,AjaxRequestTarget>) ?
The complain that I can forsee from a user is: why AjaxSubmitLink has
#onSubmit(java.lang.String,
SerializableBiConsumer<AjaxSubmitLink,AjaxRequestTarget>) but doesn't have
anything for #onError() ?
This is one of the most common problems in the forums (users@ and SO):
beginners override #onSubmit() but do not override #onError() and then
wonder why submit is not called and nothing indicates that there is an
error in the logs and/or in UI,
>
> Martijn
>
