I think we all (more or less :-) ) agree about moving factory methods
away from components. I don't think we need a dedicated and experimental
module for them. I'd rather move them to class Lamdas and, if we need
it, we can deprecate them in future.
On 08/02/2017 17:24, Sven Meier wrote:
I like Edmond's suggestion:
"We can also move them all to a factory class and place this class in
experimental.
That way we can do with it whatever we like."
So we have a place to refer questioners to when they are looking for
these factory methods.
Regards
Sven
On 08.02.2017 16:55, Martin Grigorov wrote:
https://github.com/apache/wicket/pull/211 :-)
Let's drop them all!
Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov
On Wed, Feb 8, 2017 at 4:34 PM, Emond Papegaaij
<[email protected]>
wrote:
I can live with the following:
Factory methodes should:
* accept exactly 1 lambda (and possibly an id)
* be a static method on the component or behavior they create
* be given meaningfull names
* be limited to at most 1 or 2 per type
* not pass the instance they create to the lambda
This would mean all methods should be moved from Lambdas and duplicates
like
AjaxLink.onClick should be removed.
Still, I like the idea to remove them all better. We can also move them
all to
a factory class and place this class in experimental. That way we
can do
with
it whatever we like.
Best regards,
Emond
On woensdag 8 februari 2017 16:20:59 CET Andrea Del Bene wrote:
I agree with Martijn, especially when he warns about the huge risk we
take
overusing lambdas and making API inconsistent. But I don't agree to
take
such an extreme choice like removing EVERY factory methods. Like
Martin
suggested these methods can be very useful when they are limited to
the
very basic functionality of a component (link onClick for a Link).
More in general I would say that a lambda factory method should not
take
more than one lambda as parameter.
On Wed, Feb 8, 2017 at 3:17 PM, Martin Grigorov <[email protected]>
wrote:
On Wed, Feb 8, 2017 at 3:08 PM, Martijn Dashorst <
[email protected]
wrote:
There are many things to consider when designing an API.
Usability of
the API, readability of the resulting code, maintenance burden for
the
core developers, but also performance, memory consumption and
serialization overhead.
Adding lifecycle fields to Component for any extension point will
dramatically increase the footprint of any Wicket application.
Lambda's are not a free lunch and add considerable memory and
serialization overhead to Java code.
Wicket is not only used for small demos of how fancy lambda's can
be,
but also for applications where 10's of thousands or even
millions of
users concurrently use the application. Adding a byte to the
footprint
of Component can increase the memory footprint of the application
with
10's of megabytes.
So yes we have considered adding these things but find the problem
they solve not worth the extra footprint. Furthermore the clarity of
the code does not increase when you have 3 or 4 methods that need
implementing, but those add considerable memory footprint (8 bytes
per
reference when unused, much more when used) when provided through
lambdas.
Calculation
Say you have 1 million components in memory. Say we add lambda's for
the primary life cycle events and enabled/visible flags. This is:
- onInitialize
- onEvent
- onReAdd
- onRemove
- onRender
- onConfigure
- onBeforeRender
- onBeforeRenderChildren
- onComponentTag
- onComponentTagBody
- onAfterRender
- onAfterRenderChildren
- onDetach
- isVisible
- isEnabled
This is without onFoo methods available in Link, Form etc. 15
references that may or may not be used. 15*8 = 120 bytes per
component. Now we have added 120MB to an application without anyone
even using the functionality. The cost is considerably higher when
actually using the lambda's. See the measurements I did for my talk
at
ApacheCon 2016 in Sevilla [1]
Of course we can try to minimize the memory footprint, and store the
event references in the data bucket of Component and store only
those
events that are actually used. But this will add an O(n^2) for every
request because we need to loop through the data array at each life
cycle event to find if there's a registered reference, and if so use
that. While for a single request this is not too big an issue, it
adds
up when you consider that this must be done for millions of
components
for all lifecycle events.
API design
So let's consider the case when we were to add setters for lambda's
to
Component et al. We can't remove the old way of subclassing because
that would break the world. Thus we have two ways of how to add an
onConfigure or onDetach event to a component. Which one is canon? We
already have at least 3 ways to determine whether a component is
visible or not:
- call setVisible (static visibility)
- override isVisible
- setVisibilityAllowed
- override onConfigure() and call setVisible
This has harmed the Wicket API in my opinion and is a result of us
not
understanding at the time how, when and how often visibility needed
to
be evaluated.
Adding setters for lambda's would at least double the API surface.
Furthermore you need to take into account that these setters
can't be
hidden by subclasses. So when you use a component that has
onConfigure
overridden, what happens when you setOnConfigure()?
public class FooPanel extends Panel {
public FooPanel(String id) { super(id); }
@Override
protected void onConfigure() {
super.onConfigure();
// if something set visiblity
}
}
add(new FooPanel("foo").setOnConfigure(() -> { ... do something
completely different ... });
So which one should prevail? the lambda or the overridden method? Ah
I
hear you say "rewrite the FooPanel such that it doesn't override
onConfigure". But then we have 2 onConfigure lambda's set:
public class FooPanel extends Panel {
public FooPanel(String id) {
super(id);
setOnConfigure(() -> {
/* if something set visibility */
});
}
}
add(new FooPanel("foo").setOnConfigure(() -> { ... do something
completely different ... });
So which one should prevail? Well, call both! OK, this means that we
need to store both lambdas. So each lifecycle event lambda store
needs
to be a data structure that can hold multiple lambdas, which
increases
the memory footprint even further. Then we have to issue of which
lambda to call first. The one added in the constructor of FooPanel,
or
the one that was set after construction?
This doesn't even take into account the semantics of for example the
onInitialize lifecycle event, which states that it is called when a
component is added to the hierarchy of a page (i.e. a parent-path
exists to the page). A setOnInitialize can be invoked at any moment
after construction. Therefore its semantics become even muddier than
they already are:
Label label = new Label(...);
add(label);
label.setOnInitialize(() -> { ... });
OK, so we don't add a lambda possibility for onInitialize (queue the
pull requests).
Another thing to consider is that a component builder currently can
decide to close off particular events from being overridden or
modified. For example:
public class FooPanel extends Panel {
public FooPanel(String id) { super(id); }
/** No longer overridable */
@Override
protected final void onConfigure() {
super.onConfigure();
// if something set visiblity
}
}
The component designer found it necessary to remove the ability to
override onConfigure().
However you can't remove the ability to add/set lambda's from
Component. These methods need to be public for them to be usable,
and
you can't hide those methods.
Summary
Lambdas are a great tool but should be used wisely and we should be
careful about our API and take good design into consideration
instead
of just papering our API with lambdas. There are many aspects to
consider, not just the fact that you can write 2 lines of
non-executable code less.
YES!
That's why I think the current factories we have is the best we can
give -
they are very handy for (my?!) the trivial case and they cannot be
extended
further with more functionality, it is just not practical to add more
to
them.
For more inspiration take a look at other UI libs like
JavaFX/TornadoFX,
Vaadin 8, Scalatags/Kotlinx.html, ...
Martijn
[1] http://www.slideshare.net/dashorst/whats-up-with-wicket-
8-and-java-8
On Wed, Feb 8, 2017 at 11:45 AM, Andrew Geery <
[email protected]>
wrote:
Rather than using static factory methods, would we ever consider
pushing
the lambdas into the component classes themselves?
For example, if we did this with the Button class, the change would
be:
- Add two private fields, SerializableConsumer<Button>
submitHandler,
errorHandler
- Add "setters" for these fields -- e.g., Button
submitHandler(SerializableConsumer<Button> submitHandler)
- Modify the onSubmit() and onError() methods to call the handler
methods
if they are not null
A call would be something like:
add(new Button("button").submitHandler(b ->
System.out.println("clicked"));
Obviously, it is still possible to sub-class Button and override
onSubmit(), so existing code will continue to work. However, for
code
going forward, it should be much less needed and, in my opinion,
much
clearer.
Another advantage to doing things this way is that sub-classes
inherit
these methods and there is no need to add static factory methods
for
every
sub-class (sub-classes of AjaxButton would be a better example of
this).
Thanks
Andrew
On Wed, Feb 8, 2017 at 3:42 AM, Martijn Dashorst <
[email protected]
wrote:
It is that your trivial use case is not my trivial use case and
that
we will end up with a 100,000 trivial use cases.
And no, confusion is not the big issue (though for
onsubmit+onerror
it
is) but creating a good API is hard. It takes time and
understanding.
Minimizing lines of code is not the only metric for a good API.
Just as using inheritance/annotations/generics for everything is
bad,
introducing factory methods everywhere will not age well.
These methods are trivial for anyone to implement and should we
reach
the conclusion that we actually need the factory methods in core,
it
is trivial to refactor them towards the Wicket supplied factories.
Are the factory methods in the way? Yes they are, because once we
add
them, we can't evolve them without adding many more (introduce
bloat)
or having to wait until a new major release.
Martijn
On Tue, Feb 7, 2017 at 11:40 PM, Martin Grigorov <
[email protected]>
wrote:
Hi,
These methods are factories for the trivial use cases where
nothing
else
needs to be overridden but the action method (methods, when
there
is
onError()). They do help to reduce the verbosity!
There are plenty of those cases. You can see many usages in
wicket-examples. I have used such methods from
wicketstuff-scala in
one
of
my projects in the past and I use similar ones in a project with
Kotlin
now.
A builder that provides methods like onConfigure(),
onComponentTag(),
onDetach(), ... would look really strange!
Who will use it instead of just creating a (inner) class ?!
But if those factory methods confuse core developers then they
will
be
even
more confusing to normal users :-/
0 from me.
Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov
On Tue, Feb 7, 2017 at 5:07 PM, Martijn Dashorst <
[email protected]
wrote:
All,
I want to remove all component factory methods that are added
for
lambda's.
My reasoning is that:
- removing them in 8.x (x > 0) is impossible
- adding them in 8.x (x > 0) is trivial
- we currently don't have a way to know what good idioms are
- having these factories push (new) developers to use the wrong
idiom
- factory methods don't allow for additional overriding, thus a
combinatorial explosion of API
- I tend to think that builders are better suited as component
factories
Because it is trivial to re-introduce these factories or their
replacements at a later time, I propose to remove them now and
figure
out in our actual applications what works, and what doesn't. I
also
think that just doing the conversion from W7 to W8 isn't
enough to
figure this out.
Example 1:
Button.java has two factory methods:
Button#onSubmit(String, SerializableConsumer<Button>), and
Button#onSubmit(String, SerializableConsumer<Button>,
SerializableConsumer<Button>)
It is not possible to see without resorting to parameter
naming,
hovering etc to see which functionality is bound by which
parameter.
I
find the naming confusing: onSubmit and onSubmit.
Example 2:
Behavior.java has two factory methods:
Behavior onTag(SerializableBiConsumer<Component,
ComponentTag>)
Behavior onAttribute(String name,
SerializableFunction<String, CharSequence> onAttribute)
These achieve basically the same functionality, but other life
cycle
events of Behavior don't have factory methods. NOR should they.
Example 3:
Lambdas.java has many factory methods, most of which are better
implemented by just using an anonymous inner class. For
example,
Lambdas.link: often times you need to override onconfigure or
oncomponenttag as well as onclick.
Martijn
--
Become a Wicket expert, learn from the best:
http://wicketinaction.com
--
Become a Wicket expert, learn from the best:
http://wicketinaction.com
