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
