I tend to agree with Jon on this. We thought long and hard about whether to have a separate interface and decided it was important for the following reasons:

- It makes a very clear distinction between simple models that get stored in the session and detachable models that don't and thus forces people to consider their selection of the best type of model for their particular application characteristics. Rolling everything into model just seems to blur what is currently a very clear choice

- As you would always need to subclass in order to provide the onAttach() and onDetach() methods I can't see any win in extending Model as opposed to DetatchableModel. In fact I think it is more risky because a developer may or may not remember to override both onAttach() and onDetach() if subclassing models but they can be forced to to this with detachable models so there is much less margin for error.


The question that I have is why do we have any models that are not associated with components? I would tend to consider this to be an anti-pattern. The Wicket Model/DetachableModel concept has (in my opinion) a very clear responsibility in providing the binding between Wicket components and the underlying business data or objects. Thus, using models for things other than this is outside the primary contract of the interface. I would like to see some examples of the exact use cases of why we need to use models that are not associated with components and then to provide a mechanism, guidelines or additional functionality that solves this problem without needing to alter the existing model approach.


-1 for changes to Model/DetachableModel
+1 for finding out why and stopping the anti-pattern of using models that are not associated with Wicket components


regards,
Chris


Jonathan Locke wrote:


i'm in favor of keeping the interface.
isn't the only problem just that you need to call attach() in getObject()?


for that problem, i like your solution of adding onGetObject() and onSetObject() and making getObject() final. but we can do this to DetachableModel without changing anything else.

getObject would stay overridable in Model. and it would be final in DetachableModel, forcing users to implement onGetObject(). this seems like exactly the right thing. normal models implement getObject() like they always did and DetachableModels have to implement onGetObject() so we can do the attachment magic. make sense?

or am i missing something?

Eelco Hillenius wrote:

The models keep giving problems. Though a powerfull concept, they are not perfect yet.

The problem was: as attach() was called in Component.getModel(), it was never called for models that were not coupled to components, or that were called directly (e.g. when a component holds a reference to the model directly itself).

The problem now is: as the calling of attach() is now put in AttachableModel.getObject(), the models are not attached automatically when a client overrides getObject(). So he has to call attach himself, or we should call attach from both Component.getModel() and AttachableModel.getObject().

This clearly isn't very nice either.

Maybe we should make it all much simpler. I propose:

1. Loose the interfaces. The starting point is just one Model base class that is smart enough to attach itself. Detaching still has to come from outside... I can't think of anything nice here (note that if you don't couple a model class to a component it will never be detached for you).

2. Model could look like:

public class Model extends AbstractModel
{
   private Serializable object;

   private transient boolean attached = false;

   public Model()
   {
   }

   public Model(final Serializable object)
   {
       this.object = object;
   }

   public final Object getObject()
   {
      attach();
      return doGetObject();
   }

public Object doGetObject() // or any other name; but this one is overridable
{
return object;
}


   public final void setObject(Object object)
   {
      doSetObject(object);
   }

public void doSetObject(Object object)
{
if (object != null)
{
if (!(object instanceof Serializable))
{
throw new WicketRuntimeException("Model object must be Serializable");
}
}


       setObject((Serializable)object);
   }

   public void setObject(Serializable object)
   {
       this.object = object;
   }

   public final void attach()
   {
       if (!attached)
       {
           onAttach();
           attached = true;
       }
   }

   public final void detach()
   {
       if (attached)
       {
           onDetach();
           attached = false;
       }
   }
}

Having this class would be enough for most uses. For advanced uses we can still have NestedModels etc.

I think having interfaces do not add much to the fun here. Sure it's nice for users to be able to implement models with their own root inherritence, but it's even better not to have to worry about attaching. And, as you can allways write wrappers and such, it's not that by not having interface we restrict users.

Thoughts?

Eelco


------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click _______________________________________________ Wicket-develop mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/wicket-develop



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
Wicket-develop mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/wicket-develop




------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click _______________________________________________ Wicket-develop mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/wicket-develop

Reply via email to