On Mon, Apr 3, 2017 at 9:20 AM, Emond Papegaaij <emond.papega...@topicus.nl> wrote:
> TL;DR Vote at the bottom > > On zondag 2 april 2017 00:25:12 CEST Pedro Santos wrote: > > > The major concern I have with this change is that it does not improve > > > anything. This change has impact on both the calling and implementing > > > side of detach. Neither side becomes easier. > > > > It does improve the IModel interface hierarchy by removing an unnecessary > > superinterface, and by fixing the code semantic by removing an empty > method > > that wasn't default, but was written as one. > > I do not agree here, as I see detach as an integral part of the IModel > lifecycle. But I don't > think we are going to agree on that. > > > > The caller (i.e. component) still always has to call detach on all > models > > > (now guared by an instanceof), because it can never be sure that the > > > actual implementation will or will not be detachable. > > > > No, the caller needs to call detach if the model is detachable only. > > > > > For example, if the type is MyCustomModel (which does not implement > > > IDetachable), a subclass of this type might implement IDetachable. > Also, > > > the implementation of MyCustomModel might change in the future. > > > > You are clearly generalizing a very specific use case, you can't say > "still > > always has to call detach". In this specific use case, where you have a > > polymorphic type, yes you do have to check if the instance is detachable. > > This occurs in Wicket's core because it's the place where generic model > > wrappers abstractions are defined and provided for the final user. If > > Wicket's core abstractions do their job right, this complexity should be > > taken away from the final user. > > Types in Java can always be polymorphic (except when they are final). A > well written > component does not depend on the specifics of its models. Therefore a > component should > be built on IModels *and* detach these model. That's the only way it will > work for all types > of models. > > > What do those models have in common? They are all wrappers to other > models. > > If we pass through this proposal, we will be able to go ahead and propose > > to remove their IDetachable interface; that was wrongfully ( and > > unthoughtful because it was forced via IModel ) add as their > superinterface. > > > > > On the side of the implementation for new models not much changes. You > > > don't need to implement detach() when you don't need it. > > > > No, you *do need*, and you *already implemented* it by choosing to > > implement IModel. You clearly mean here that you successfully hide from > > the user an empty method, written with a semantically wrong word, > enforced > > by a contract that shouldn't be there in the first place. It's just > > trickery, not a good interface not enforcing an unneeded method. > > What I mean here is that you do not have to write any code. > > > > If you do need it, you now > > > need 2 changes: implement IDetachable *and* implement detach(). > > > > No, a detachable model *always needed* to implement IDetacheble > interface, > > no changes here. > > What I mean here is that the author of a model that requires detach() now > also has to add > 'implements IDetachable'. This was not needed before, because it was > inherited from > IModel. > > I think we are not going to agree on this proposal. I think it is not an > improvement and I do > not agree with you that IModel should not be detachable by default. So > lets vote on this. > > [ ] Yes, remove IDetachable from IModel You don't give us any options ;-)
