Rather than calling the class Lambdas (how the components are created), could we call it Components (what is created)? This would make it more like j.u.Collections, j.u.Objects and j.u.s.Collectors...
I think the real issue here is the problem of naming the factory methods. Going back to the original concern with the Button class, the issue seems not so much the use of lambdas but the fact that an error handler lambda is being passed into a method called onSubmit. I think either we should remove the method with the onError handler lambda or change its name to something more appropriate (e.g., just button(...)). Andrew On Thu, Feb 9, 2017 at 5:23 AM, Vít Rozkovec <[email protected]> wrote: > Would it be possible to organize Lambdas in a way to be able to invoke it > as such: > > Lambdas.Behavior.onConfigure() > Lambdas.Link.onClick() > > Wouldn't it also help in the future in case of refactoring? > > > > > On 02/08/2017 08:53 PM, Andrea Del Bene wrote: > >> 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 >>>>> >>>>> >>>>> >>>>> >>> >> >> >
