You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Eelco Hillenius <ee...@gmail.com> on 2007/08/27 08:45:54 UTC
model wrapper for ReuseIfModelsEqualStrategy or alternative reuse strategy?
Hi,
For Wicket In Action in converted an example from a ListView to a
RepeatingView. This example has to function in a form, so I set the
'ReuseIfModelsEqualStrategy'. Overriding hashCode (and equals) on the
model however results in quite a bit of code bloat, while my hunch is
that comparing the model values is actually a pretty common use case.
So, in my quest to cut down a few lines, I would basically have two
choices: implement a reuse strategy that does this, or implement a
model wrapper that implements hashCode and equals in a generic
fashion. I choose the latter, and would like to hear your opinions
whether this is something we could add to the project (I think the use
case for it should be common enough, especially since I would mention
it in the book), or whether you would prefer to have a reuse strategy
that does this.
I'd really like to see one of these in the project, as otherwise I
would have to either include the equals/ hashCode implementation
inline, which bloats the example, or I would have to give the code in
the book as people will want to play with it. Both would suck.
Cheers,
Eelco
My implementation (no javadocs):
package wicket.in.action.common;
import java.io.ObjectStreamException;
import java.io.Serializable;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import org.apache.wicket.model.IModel;
import org.apache.wicket.util.lang.Objects;
public final class EqualsDecorator {
private EqualsDecorator() {
}
public static IModel decorate(final IModel model) {
return (IModel) Proxy.newProxyInstance(model.getClass()
.getClassLoader(), model.getClass().getInterfaces(),
new Decorator(model));
}
private static class Decorator implements InvocationHandler,
Serializable {
private final IModel model;
Decorator(IModel model) {
this.model = model;
}
public Object invoke(Object proxy, Method method, Object[] args)
throws Throwable {
String methodName = method.getName();
if (methodName.equals("equals")) {
if (args[0] instanceof IModel) {
return Objects.equal(model.getObject(), ((IModel) args[0])
.getObject());
}
} else if (methodName.equals("hashCode")) {
Object val = model.getObject();
return 37 + (val != null ? val.hashCode() : 0);
} else if (methodName.equals("writeReplace")) {
return new SerializableReplacement(model);
}
return method.invoke(model, args);
}
}
private static class SerializableReplacement implements
Serializable {
private final IModel model;
SerializableReplacement(IModel model) {
this.model = model;
}
private Object readResolve() throws ObjectStreamException {
return decorate(model);
}
}
}
Re: model wrapper for ReuseIfModelsEqualStrategy or alternative reuse strategy?
Posted by Igor Vaynberg <ig...@gmail.com>.
i dont know if we want to go down this road. its a mixin that is not very
obvious, how many people do you think know about IStyledColumn? besides it
might be two objects that represent the pk - like two longs for composite
pks. i really dont think it is that big a deal to implement equals/hashcode
on a model, and if people have their own variants of entitymodels they only
have one place to implement it. imho this is just bloat.
-igor
On 8/27/07, Matej Knopp <ma...@gmail.com> wrote:
>
> Perhaps IEntityModel Object getEntityId/PK() would be better name?
> ICompressedModel doesn't really say much, sounds too abstract to me. Also
> I
> could live with EqualsDecorator as long as it clearly states with CAPITAL
> LETTERS that it can't be used on detachable models :)
>
> -Matej
>
> On 8/27/07, Eelco Hillenius <ee...@gmail.com> wrote:
> >
> > On 8/27/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > > the reason i did not provide this when i wrote the repeaters package
> is
> > that
> > > it can lead down some pretty awful roads. an easy example where this
> > goes
> > > totally wrong is when you use detachable models for the items. if you
> > use
> > > this decorator all models from the previous page will be loaded
> because
> > they
> > > will all be compared - but in case of the detachable model it is not
> > > necesary to load the object, it is enough to only compare the id.
> > > so while this makes it easy it also makes it easy to make something
> > > inefficient because you dont have to think about it.
> >
> > Ok, I can follow that. If you are not using such models however, this
> > is a very convenient class. I think that if we give a clear enough
> > warning in the docs (and book) it should be ok.
> >
> > How about facilitating the case you described a little bit better as
> > well? Every project probably has it's own variants of entity models by
> > now. Wouldn't it be an idea to introduce an interface like
> > ICompressedModel { Object getCompressed() } or something along those
> > lines that people can implement and which would return e.g. the id
> > which would then be used by the wrapper?
> >
> > Eelco
> >
>
Re: model wrapper for ReuseIfModelsEqualStrategy or alternative reuse strategy?
Posted by Matej Knopp <ma...@gmail.com>.
Perhaps IEntityModel Object getEntityId/PK() would be better name?
ICompressedModel doesn't really say much, sounds too abstract to me. Also I
could live with EqualsDecorator as long as it clearly states with CAPITAL
LETTERS that it can't be used on detachable models :)
-Matej
On 8/27/07, Eelco Hillenius <ee...@gmail.com> wrote:
>
> On 8/27/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > the reason i did not provide this when i wrote the repeaters package is
> that
> > it can lead down some pretty awful roads. an easy example where this
> goes
> > totally wrong is when you use detachable models for the items. if you
> use
> > this decorator all models from the previous page will be loaded because
> they
> > will all be compared - but in case of the detachable model it is not
> > necesary to load the object, it is enough to only compare the id.
> > so while this makes it easy it also makes it easy to make something
> > inefficient because you dont have to think about it.
>
> Ok, I can follow that. If you are not using such models however, this
> is a very convenient class. I think that if we give a clear enough
> warning in the docs (and book) it should be ok.
>
> How about facilitating the case you described a little bit better as
> well? Every project probably has it's own variants of entity models by
> now. Wouldn't it be an idea to introduce an interface like
> ICompressedModel { Object getCompressed() } or something along those
> lines that people can implement and which would return e.g. the id
> which would then be used by the wrapper?
>
> Eelco
>
Re: model wrapper for ReuseIfModelsEqualStrategy or alternative reuse strategy?
Posted by Eelco Hillenius <ee...@gmail.com>.
On 8/27/07, Igor Vaynberg <ig...@gmail.com> wrote:
> the reason i did not provide this when i wrote the repeaters package is that
> it can lead down some pretty awful roads. an easy example where this goes
> totally wrong is when you use detachable models for the items. if you use
> this decorator all models from the previous page will be loaded because they
> will all be compared - but in case of the detachable model it is not
> necesary to load the object, it is enough to only compare the id.
> so while this makes it easy it also makes it easy to make something
> inefficient because you dont have to think about it.
Ok, I can follow that. If you are not using such models however, this
is a very convenient class. I think that if we give a clear enough
warning in the docs (and book) it should be ok.
How about facilitating the case you described a little bit better as
well? Every project probably has it's own variants of entity models by
now. Wouldn't it be an idea to introduce an interface like
ICompressedModel { Object getCompressed() } or something along those
lines that people can implement and which would return e.g. the id
which would then be used by the wrapper?
Eelco
Re: model wrapper for ReuseIfModelsEqualStrategy or alternative reuse strategy?
Posted by Igor Vaynberg <ig...@gmail.com>.
the reason i did not provide this when i wrote the repeaters package is that
it can lead down some pretty awful roads. an easy example where this goes
totally wrong is when you use detachable models for the items. if you use
this decorator all models from the previous page will be loaded because they
will all be compared - but in case of the detachable model it is not
necesary to load the object, it is enough to only compare the id.
so while this makes it easy it also makes it easy to make something
inefficient because you dont have to think about it.
+0
-igor
On 8/26/07, Eelco Hillenius <ee...@gmail.com> wrote:
>
> Hi,
>
> For Wicket In Action in converted an example from a ListView to a
> RepeatingView. This example has to function in a form, so I set the
> 'ReuseIfModelsEqualStrategy'. Overriding hashCode (and equals) on the
> model however results in quite a bit of code bloat, while my hunch is
> that comparing the model values is actually a pretty common use case.
>
> So, in my quest to cut down a few lines, I would basically have two
> choices: implement a reuse strategy that does this, or implement a
> model wrapper that implements hashCode and equals in a generic
> fashion. I choose the latter, and would like to hear your opinions
> whether this is something we could add to the project (I think the use
> case for it should be common enough, especially since I would mention
> it in the book), or whether you would prefer to have a reuse strategy
> that does this.
>
> I'd really like to see one of these in the project, as otherwise I
> would have to either include the equals/ hashCode implementation
> inline, which bloats the example, or I would have to give the code in
> the book as people will want to play with it. Both would suck.
>
> Cheers,
>
> Eelco
>
> My implementation (no javadocs):
>
> package wicket.in.action.common;
>
> import java.io.ObjectStreamException;
> import java.io.Serializable;
> import java.lang.reflect.InvocationHandler;
> import java.lang.reflect.Method;
> import java.lang.reflect.Proxy;
>
> import org.apache.wicket.model.IModel;
> import org.apache.wicket.util.lang.Objects;
>
> public final class EqualsDecorator {
>
> private EqualsDecorator() {
> }
>
> public static IModel decorate(final IModel model) {
> return (IModel) Proxy.newProxyInstance(model.getClass()
> .getClassLoader(), model.getClass().getInterfaces(),
> new Decorator(model));
> }
>
> private static class Decorator implements InvocationHandler,
> Serializable {
>
> private final IModel model;
>
> Decorator(IModel model) {
> this.model = model;
> }
>
> public Object invoke(Object proxy, Method method, Object[] args)
> throws Throwable {
> String methodName = method.getName();
> if (methodName.equals("equals")) {
> if (args[0] instanceof IModel) {
> return Objects.equal(model.getObject(), ((IModel) args[0])
> .getObject());
> }
> } else if (methodName.equals("hashCode")) {
> Object val = model.getObject();
> return 37 + (val != null ? val.hashCode() : 0);
> } else if (methodName.equals("writeReplace")) {
> return new SerializableReplacement(model);
> }
> return method.invoke(model, args);
> }
> }
>
> private static class SerializableReplacement implements
> Serializable {
> private final IModel model;
>
> SerializableReplacement(IModel model) {
> this.model = model;
> }
>
> private Object readResolve() throws ObjectStreamException {
> return decorate(model);
> }
> }
> }
>