Re: [wicket8] Use default method for IModel#detach()?

2017-03-27 Thread Sven Meier

Hi Pedro,

>Why don't we make AbstractReadyOnlyModel the superinterface of IModel

IIRC Michael Mosmann tried to work on something similar but it never 
quite worked out.


Do you want to work on a proposal?

Before you invest too much time into this: I'm pretty sure most devs 
won't be in favor of doing another round of IModel refactoring in Wicket 
8 because of semantics.


Regards
Sven


On 27.03.2017 01:39, Pedro Santos wrote:

-0

I see no good reason for IModel to extend from IDetachable. Users should be
able to add this interface at their will.


If IModel were a @FunctionalInterface, then you wouldn't need something like

a SupplierModel; you could just use a lambda directly:

IModel, as it's now, isn't a functional interface.


Then maybe just make IModel#setObject() default too ..

There's no default implementation for IModel#setObject(). The one that was
added is as much semantically wrong as to say IModel is a functional
interface.

Proposal:

Why don't we make AbstractReadyOnlyModel the superinterface of IModel
instead of to keep it as an abstract adaptor for IModel? So we would have a
semantically correct functional interface.



Pedro Santos

On Tue, Oct 6, 2015 at 6:42 PM, Martin Grigorov 
wrote:


Ugh, right!

Then maybe just make IModel#setObject() default too ...
We will experiment! :)

Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov

On Tue, Oct 6, 2015 at 11:27 PM, Michael Mosmann 
wrote:


.. @IReadOnlyModel: i hope, it will be easier to change then the last

time

i tried it. Watch out for component default model stuff...

:)

Am 6. Oktober 2015 22:54:37 MESZ, schrieb Martin Grigorov <
mgrigo...@apache.org>:

On Tue, Oct 6, 2015 at 10:43 PM, Andrew Geery 
wrote:


If IModel were a @FunctionalInterface, then you wouldn't need

something

like a SupplierModel; you could just use a lambda directly:

new Label("label", () -> "The current time is " + LocalDate.now());


This looks good indeed.
It seems we will add IReadOnlyModel soon!



And since IModel is Serializable, the lambda will be too, without

having to

have an artificial interface that is both Serializable & a Supplier.

Thanks
Andrew

On Tue, Oct 6, 2015 at 4:21 PM, Martin Grigorov



wrote:


Same for IRequestHandler#detach()

Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov

On Mon, Oct 5, 2015 at 10:42 PM, Martijn Dashorst <
martijn.dasho...@gmail.com> wrote:


Should we use an empty default implementation for IModel#detach?


public class IModel extends IDetachable
{
 ...

 @Override
 default void detach()
 {
 }
}

This won't break existing applications, but might make it a bit

easier

on the eyes to implement IModel directly.

I'm not in favor of applying the default method to IDetachable,
because that would defeat the interface's purpose IMO.

WDYT?

Martijn


--
Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail
gesendet.




Re: wicket git commit: WICKET-6347 added #detach() to all renderers

2017-03-27 Thread Martin Grigorov
Hi Sven,

One minor issue: the renderer is optional (i.e. null-able) in some classes,
so you will have to check before calling its #detach() method. For example
in AjaxEditableChoiceLabel

Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov

On Mon, Mar 27, 2017 at 5:28 PM,  wrote:

> Repository: wicket
> Updated Branches:
>   refs/heads/WICKET-6347-detachable-renderers [created] 0bc929d71
>
>
> WICKET-6347 added #detach() to all renderers
>
> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/0bc929d7
> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/0bc929d7
> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/0bc929d7
>
> Branch: refs/heads/WICKET-6347-detachable-renderers
> Commit: 0bc929d71865f052e895fd0918ea297acc7086fa
> Parents: 3179d34
> Author: Sven Meier 
> Authored: Mon Mar 27 17:27:20 2017 +0200
> Committer: Sven Meier 
> Committed: Mon Mar 27 17:27:20 2017 +0200
>
> --
>  .../wicket/markup/html/form/AbstractChoice.java  |  8 
>  .../wicket/markup/html/form/IChoiceRenderer.java | 15 ---
>  .../ajax/markup/html/AjaxEditableChoiceLabel.java|  3 +++
>  .../html/autocomplete/AutoCompleteBehavior.java  |  7 +++
>  .../html/autocomplete/AutoCompleteTextField.java |  8 
>  .../html/autocomplete/IAutoCompleteRenderer.java | 11 +--
>  .../extensions/markup/html/form/palette/Palette.java |  2 ++
>  .../markup/html/form/select/IOptionRenderer.java | 12 ++--
>  .../markup/html/form/select/SelectOptions.java   |  8 
>  9 files changed, 67 insertions(+), 7 deletions(-)
> --
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/
> 0bc929d7/wicket-core/src/main/java/org/apache/wicket/markup/
> html/form/AbstractChoice.java
> --
> diff --git 
> a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/AbstractChoice.java
> b/wicket-core/src/main/java/org/apache/wicket/markup/html/
> form/AbstractChoice.java
> index 523116e..fd1cacf 100644
> --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/
> form/AbstractChoice.java
> +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/
> form/AbstractChoice.java
> @@ -501,4 +501,12 @@ public abstract class AbstractChoice extends
> FormComponent
> "This class does not support type-conversion
> because it is performed "
> + "exclusively by the IChoiceRenderer
> assigned to this component");
> }
> +
> +   @Override
> +   protected void onDetach()
> +   {
> +   renderer.detach();
> +
> +   super.onDetach();
> +   };
>  }
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/
> 0bc929d7/wicket-core/src/main/java/org/apache/wicket/markup/
> html/form/IChoiceRenderer.java
> --
> diff --git 
> a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/IChoiceRenderer.java
> b/wicket-core/src/main/java/org/apache/wicket/markup/html/
> form/IChoiceRenderer.java
> index c8345d4..5fcc14e 100644
> --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/
> form/IChoiceRenderer.java
> +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/
> form/IChoiceRenderer.java
> @@ -18,8 +18,8 @@ package org.apache.wicket.markup.html.form;
>
>  import java.util.List;
>
> +import org.apache.wicket.model.IDetachable;
>  import org.apache.wicket.model.IModel;
> -import org.apache.wicket.util.io.IClusterable;
>
>  /**
>   * Renders one choice. Separates the 'id' values used for internal
> representation from 'display
> @@ -30,7 +30,7 @@ import org.apache.wicket.util.io.IClusterable;
>   * @param 
>   *The model object type
>   */
> -public interface IChoiceRenderer extends IClusterable
> +public interface IChoiceRenderer extends IDetachable
>  {
> /**
>  * Get the value for displaying to an end user.
> @@ -70,4 +70,13 @@ public interface IChoiceRenderer extends IClusterable
>  * @return A choice from the list that has this {@code id}
>  */
> T getObject(String id, IModel>
> choices);
> -}
> +
> +   /**
> +* Override when needed.
> +*/
> +   @Override
> +   default void detach()
> +   {
> +   }
> +
> +}
> \ No newline at end of file
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/
> 0bc929d7/wicket-extensions/src/main/java/org/apache/
> wicket/extensions/ajax/markup/html/AjaxEditableChoiceLabel.java
> --
> diff --git 

Re: [wicket8] Use default method for IModel#detach()?

2017-03-27 Thread Pedro Santos
Neither I am sure I understand you. By dating the email thread and the
IModel interface hierarchy you mean that by them being old we can't discuss
their merit?

I meant that IModel isn't a functional interface, not that it isn't
annotated as one. Same for the setObject method not having a default
implementation.

IModel is the interface of objects with two behaviours, they can set and
get model objects. A functional interface is a contract promising only one
behavior.

How to throw an exception is the default functionality of setObject method?
Nothing is being set there, so it doesn't fit as a default implementation.

On Mar 27, 2017 3:22 AM, "Martin Grigorov"  wrote:

Hi Pedro,

I am not sure I understand you!
This discussion was 1.5 years ago ...

On Mon, Mar 27, 2017 at 1:39 AM, Pedro Santos  wrote:

> -0
>
> I see no good reason for IModel to extend from IDetachable. Users should
be
> able to add this interface at their will.
>

IModel extends IDetachable since forever
https://github.com/apache/wicket/blob/wicket-1.3.x/jdk-
1.4/wicket/src/main/java/org/apache/wicket/model/IModel.java#L56


>
> >If IModel were a @FunctionalInterface, then you wouldn't need something
> like
> a SupplierModel; you could just use a lambda directly:
>
> IModel, as it's now, isn't a functional interface.
>

Yes, it is!
https://github.com/apache/wicket/blob/8a4e1b3c24f4ce1fc3e44ca1b5a923
0158d9b584/wicket-core/src/main/java/org/apache/wicket/model/IModel.java#L63


>
> >Then maybe just make IModel#setObject() default too ..
>
> There's no default implementation for IModel#setObject(). The one that was
>

Yes, there is!
https://github.com/apache/wicket/blob/8a4e1b3c24f4ce1fc3e44ca1b5a923
0158d9b584/wicket-core/src/main/java/org/apache/wicket/model/IModel.java#L81


> added is as much semantically wrong as to say IModel is a functional
> interface.



> Proposal:
>
> Why don't we make AbstractReadyOnlyModel the superinterface of IModel
> instead of to keep it as an abstract adaptor for IModel? So we would have
a
> semantically correct functional interface.
>

IMO the current design looks and works quite good! No need to change
anything!


>
>
>
> Pedro Santos
>
> On Tue, Oct 6, 2015 at 6:42 PM, Martin Grigorov 
> wrote:
>
> > Ugh, right!
> >
> > Then maybe just make IModel#setObject() default too ...
> > We will experiment! :)
> >
> > Martin Grigorov
> > Wicket Training and Consulting
> > https://twitter.com/mtgrigorov
> >
> > On Tue, Oct 6, 2015 at 11:27 PM, Michael Mosmann 
> > wrote:
> >
> > > .. @IReadOnlyModel: i hope, it will be easier to change then the last
> > time
> > > i tried it. Watch out for component default model stuff...
> > >
> > > :)
> > >
> > > Am 6. Oktober 2015 22:54:37 MESZ, schrieb Martin Grigorov <
> > > mgrigo...@apache.org>:
> > > >On Tue, Oct 6, 2015 at 10:43 PM, Andrew Geery  >
> > > >wrote:
> > > >
> > > >> If IModel were a @FunctionalInterface, then you wouldn't need
> > > >something
> > > >> like a SupplierModel; you could just use a lambda directly:
> > > >>
> > > >> new Label("label", () -> "The current time is " + LocalDate.now());
> > > >>
> > > >
> > > >This looks good indeed.
> > > >It seems we will add IReadOnlyModel soon!
> > > >
> > > >
> > > >>
> > > >> And since IModel is Serializable, the lambda will be too, without
> > > >having to
> > > >> have an artificial interface that is both Serializable & a
Supplier.
> > > >>
> > > >> Thanks
> > > >> Andrew
> > > >>
> > > >> On Tue, Oct 6, 2015 at 4:21 PM, Martin Grigorov
> > > >
> > > >> wrote:
> > > >>
> > > >> > Same for IRequestHandler#detach()
> > > >> >
> > > >> > Martin Grigorov
> > > >> > Wicket Training and Consulting
> > > >> > https://twitter.com/mtgrigorov
> > > >> >
> > > >> > On Mon, Oct 5, 2015 at 10:42 PM, Martijn Dashorst <
> > > >> > martijn.dasho...@gmail.com> wrote:
> > > >> >
> > > >> > > Should we use an empty default implementation for
IModel#detach?
> > > >> > >
> > > >> > >
> > > >> > > public class IModel extends IDetachable
> > > >> > > {
> > > >> > > ...
> > > >> > >
> > > >> > > @Override
> > > >> > > default void detach()
> > > >> > > {
> > > >> > > }
> > > >> > > }
> > > >> > >
> > > >> > > This won't break existing applications, but might make it a bit
> > > >easier
> > > >> > > on the eyes to implement IModel directly.
> > > >> > >
> > > >> > > I'm not in favor of applying the default method to IDetachable,
> > > >> > > because that would defeat the interface's purpose IMO.
> > > >> > >
> > > >> > > WDYT?
> > > >> > >
> > > >> > > Martijn
> > > >> > >
> > > >> >
> > > >>
> > >
> > > --
> > > Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail
> > > gesendet.
> >
>


Re: [wicket8] Use default method for IModel#detach()?

2017-03-27 Thread Martin Grigorov
Hi Pedro,

I am not sure I understand you!
This discussion was 1.5 years ago ...

On Mon, Mar 27, 2017 at 1:39 AM, Pedro Santos  wrote:

> -0
>
> I see no good reason for IModel to extend from IDetachable. Users should be
> able to add this interface at their will.
>

IModel extends IDetachable since forever
https://github.com/apache/wicket/blob/wicket-1.3.x/jdk-1.4/wicket/src/main/java/org/apache/wicket/model/IModel.java#L56


>
> >If IModel were a @FunctionalInterface, then you wouldn't need something
> like
> a SupplierModel; you could just use a lambda directly:
>
> IModel, as it's now, isn't a functional interface.
>

Yes, it is!
https://github.com/apache/wicket/blob/8a4e1b3c24f4ce1fc3e44ca1b5a9230158d9b584/wicket-core/src/main/java/org/apache/wicket/model/IModel.java#L63


>
> >Then maybe just make IModel#setObject() default too ..
>
> There's no default implementation for IModel#setObject(). The one that was
>

Yes, there is!
https://github.com/apache/wicket/blob/8a4e1b3c24f4ce1fc3e44ca1b5a9230158d9b584/wicket-core/src/main/java/org/apache/wicket/model/IModel.java#L81


> added is as much semantically wrong as to say IModel is a functional
> interface.



> Proposal:
>
> Why don't we make AbstractReadyOnlyModel the superinterface of IModel
> instead of to keep it as an abstract adaptor for IModel? So we would have a
> semantically correct functional interface.
>

IMO the current design looks and works quite good! No need to change
anything!


>
>
>
> Pedro Santos
>
> On Tue, Oct 6, 2015 at 6:42 PM, Martin Grigorov 
> wrote:
>
> > Ugh, right!
> >
> > Then maybe just make IModel#setObject() default too ...
> > We will experiment! :)
> >
> > Martin Grigorov
> > Wicket Training and Consulting
> > https://twitter.com/mtgrigorov
> >
> > On Tue, Oct 6, 2015 at 11:27 PM, Michael Mosmann 
> > wrote:
> >
> > > .. @IReadOnlyModel: i hope, it will be easier to change then the last
> > time
> > > i tried it. Watch out for component default model stuff...
> > >
> > > :)
> > >
> > > Am 6. Oktober 2015 22:54:37 MESZ, schrieb Martin Grigorov <
> > > mgrigo...@apache.org>:
> > > >On Tue, Oct 6, 2015 at 10:43 PM, Andrew Geery  >
> > > >wrote:
> > > >
> > > >> If IModel were a @FunctionalInterface, then you wouldn't need
> > > >something
> > > >> like a SupplierModel; you could just use a lambda directly:
> > > >>
> > > >> new Label("label", () -> "The current time is " + LocalDate.now());
> > > >>
> > > >
> > > >This looks good indeed.
> > > >It seems we will add IReadOnlyModel soon!
> > > >
> > > >
> > > >>
> > > >> And since IModel is Serializable, the lambda will be too, without
> > > >having to
> > > >> have an artificial interface that is both Serializable & a Supplier.
> > > >>
> > > >> Thanks
> > > >> Andrew
> > > >>
> > > >> On Tue, Oct 6, 2015 at 4:21 PM, Martin Grigorov
> > > >
> > > >> wrote:
> > > >>
> > > >> > Same for IRequestHandler#detach()
> > > >> >
> > > >> > Martin Grigorov
> > > >> > Wicket Training and Consulting
> > > >> > https://twitter.com/mtgrigorov
> > > >> >
> > > >> > On Mon, Oct 5, 2015 at 10:42 PM, Martijn Dashorst <
> > > >> > martijn.dasho...@gmail.com> wrote:
> > > >> >
> > > >> > > Should we use an empty default implementation for IModel#detach?
> > > >> > >
> > > >> > >
> > > >> > > public class IModel extends IDetachable
> > > >> > > {
> > > >> > > ...
> > > >> > >
> > > >> > > @Override
> > > >> > > default void detach()
> > > >> > > {
> > > >> > > }
> > > >> > > }
> > > >> > >
> > > >> > > This won't break existing applications, but might make it a bit
> > > >easier
> > > >> > > on the eyes to implement IModel directly.
> > > >> > >
> > > >> > > I'm not in favor of applying the default method to IDetachable,
> > > >> > > because that would defeat the interface's purpose IMO.
> > > >> > >
> > > >> > > WDYT?
> > > >> > >
> > > >> > > Martijn
> > > >> > >
> > > >> >
> > > >>
> > >
> > > --
> > > Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail
> > > gesendet.
> >
>