Re: [infinispan-dev] QueryIterator inconsistencies

On 25 September 2012 15:53, Manik Surtani <manik(a)jboss.org&gt; wrote: > > On 25 Sep 2012, at 15:00, Sanne Grinovero <sanne(a)infinispan.org&gt; wrote: > > On 25 September 2012 12:11, Marko Lukša <marko.luksa(a)gmail.com&gt; wrote: > > On 25.9.2012 11:51, Sanne Grinovero wrote: > > Any comments / other ideas? > > Marko > > That's a lot of changes needed. One would say we could deprecate all > methods and rewrite the new ones, but wouldn't it be better to > deprecate the interface and introduce a brand new one? > What about "ResultsIterator" ? after all, we're not iterating on queries.. > > I agree. The only thing that bothers me is that CacheQuery would then > need new methods called (lazy)resultIterator() (instead of iterator(), > which already exists). > > Right. though I like "resultIterator()" more so we're having luck in > this case ;) > > Any better suggestion for names? Anyone thinks it's worth to remove > the old ones without a deprecation step to reuse the old names? > > One thing to keep in mind is that CacheQuery extends Iterable, therefore > a lot of users will actually implicitly invoke the iterator() method. > This pretty much means we should not introduce resultIterator(). > > > Looks like we have to break backwards compatibility. From the look of > how this API was, I guess not many users dealt with it before you, so > I'm fine with that, I don't see how else we can fix this API. > > While at it, it would be nice to see if we can change the signature > from "extends Iterable<Object>" to return an iterator on the types > compatible with the values of the cache, since the Cache supports > proper generics. > > To deal safely with Iterable, I think it would be better to default to > lazy loading mode and to provide an overloaded method which allows to > specify a loading mode. > > Anyone having a better idea which allows to not break backwards > compatibility? > > > Not really - I think compatibility will break, sadly. :/ > > One option to maintain compatibility is to keep the existing QueryIterator > and mark it as deprecated, and write a new iterator, maybe call it a > ResultIterator (which is more accurate anyway!). Your screen is too small ;-)

That was suggested above ^^ but isn't possible as a) we can't change the method name as it would break the fact CacheQuery implements Iterable

b) we can't change the return type of the existing method name

> > -- > Manik Surtani > manik(a)jboss.org > twitter.com/maniksurtani > > Platform Architect, JBoss Data Grid > http://red.ht/data-grid > > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev