https://gwt-code-reviews.appspot.com/1646803/diff/1/user/src/com/google/web/bindery/requestfactory/server/Resolver.java File user/src/com/google/web/bindery/requestfactory/server/Resolver.java (right):
https://gwt-code-reviews.appspot.com/1646803/diff/1/user/src/com/google/web/bindery/requestfactory/server/Resolver.java#newcode365 user/src/com/google/web/bindery/requestfactory/server/Resolver.java:365: new IdentityHashMap<BaseProxy, Resolution>(); On 2012/05/29 17:08:02, rdayal wrote:
On 2012/05/27 08:41:35, tbroyer wrote: > On 2012/05/23 01:08:29, rdayal wrote: > > Sorry for the long delay on this, but doing a bit of thinking - it
seems
> > incorrect to add an empty ValueProxy object to the clientObjectToResolutionMap > > when there's no representation of it. It seems that items should
be added to
> > this list when they're actually resolved (i.e filled in; not
empty).
> > > > Can we come up with a solution that satisfies this? It may require
the
> addition > > of another "unresolved" List, and then migration over to the map
when items
> are > > filled in. > > I'll have to think more about it... > > > Though it's going to add more code, I think the workaround we have
here is a
> bit > > of a hack. I think it's an elegant hack, but it just feels wrong
to be
working > > around ValueProxy.equals(....) because we're trying to add empty
value
proxies > > to a "resolved" map when they really haven't been totally resolved
yet.
> > > > Thoughts? > > I really think we _need_ an IdentityHashMap here, whether we add an "unresolved" > list or not. We want to map proxy instances to their "resolution"
object, we
> know we will be modifying the proxies, so a Map that uses equals()
is a
mistake: > a) you shouldn't use mutable objects as keys, and b) we don't care
about
objects > that compare equal, we want to map a very specific instance (be it
an entity
> proxy or value proxy, it really doesn't matter) to a Resolution
object.
>
Your justification to use an IdentityHashMap does make sense. It just
strikes me
as odd that we're using a Map where the key is mutable (in general,
regardless
of whether or not this is an IdentityHashMap). It just seems that
there must be
a better way to do this. I'd be happier if the key was some sort of
synthetic ID
that was generated at creation time (of the ValueProxy or
EntityProxy), and that
key mapped to both the resolution object and the proxy object.
In the interest of expediency, we can defer "fixing" this issue. I'd request that we add some doc to your existing comment indicating that this map is strange in that the key itself is mutated. https://gwt-code-reviews.appspot.com/1646803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
