Re: [infinispan-dev] QueryIterator inconsistencies

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?