Paulo Gaspar wrote:
>
> In Foreach.getIterator(), I don't "get" several things:
> - Why not accepting an object of type Iterator?
Where? I don't understand.
> - Why checking sizes to return null has in
> if (((Collection) listObject).size() == 0)
> return null;
> ???
> That seems to be one of the forces behind inconsistencies
> like the already criticized case of casting to Collection
> (just to mentioned the INFO_ITERATOR type) of an object
> that only had to provide an iterator() method in the
> initial check. The code would work anyway with an empty
> iterator and, for speed, it is more correct to make a
> hasNext() check to the resulting iterator. (Anyway, for
> speed, such check would only be efficient if there are
> many empty #foreach arguments in typical use.)
I am not sure if I completely follow this. I don't remember the casting
to a collection bit, but I will go review.
I think there could be an issue there - you are right. We should do a
real check rather than the null hack.
> - Why caching the Introspection information based in the
> #foreach directive object as in
> IntrospectionCacheData icd = context.icacheGet( this );
> ???
> It makes more sense to cache what is the right iterator
> based on the listObject class, since the class of the
> listObject might change between uses of the same template,
> but the found iterator will always be the same for each
> listObject class.
You are missing something, I think. That problem, the class changing,
is recognized, so we cache what we find until the class of the object
changes.
There is even a test in the testbed to check this behavior,
ContextSafetyTestCase.java. I will doublecheck if you think it isn't
working.
> I am putting my hands on this, but just want to check if I am
> missing something.
What does "I am putting my hands on this..." mean?
>
> Have fun... and say something,
> Paulo Gaspar
Aside from the caching issue, which I think you missed, if there are
real problems here, give it a whack. Are there any use problems?
geir
--
Geir Magnusson Jr. [EMAIL PROTECTED]
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity
