You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by Paulo Gaspar <pa...@krankikom.de> on 2001/02/14 16:42:33 UTC

Iterator introspection nonsense?

In Foreach.getIterator(), I don't "get" several things:
  - Why not accepting an object of type Iterator?

  - 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.)

  - 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.


I am putting my hands on this, but just want to check if I am
missing something.


Have fun... and say something,
Paulo Gaspar

Re: Iterator introspection nonsense?

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
jeff wrote:
> 
> At 12:17 PM 2/14/01 -0500, you wrote:
> >Paulo Gaspar wrote:
> >>
> >> Answer inline:
> >>
> >> > -----Original Message-----
> >> > From: gmj@mta4.srv.hcvlny.cv.net [mailto:gmj@mta4.srv.hcvlny.cv.net]On
> >> > Behalf Of Geir Magnusson Jr.
> >> >
> >> > 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.
> >>
> >> Just accepting an Iterator as the object to iterate trough in the
> >> #foreach.
> >
> >Yes. Ok. Agreed.
> 
> How about Enumeration too, while you're at it.  I know it's deprecated but
> it's still in a lot of APIs, including the Servlet API.

Hm.  I wasn't aware it was deprecated.  Learn someting new every day.

I have no objection : the more we do the better, I think.   What else?

geir

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

Re: Iterator introspection nonsense?

Posted by jeff <jl...@houseofdistraction.com>.
At 12:17 PM 2/14/01 -0500, you wrote:
>Paulo Gaspar wrote:
>> 
>> Answer inline:
>> 
>> > -----Original Message-----
>> > From: gmj@mta4.srv.hcvlny.cv.net [mailto:gmj@mta4.srv.hcvlny.cv.net]On
>> > Behalf Of Geir Magnusson Jr.
>> >
>> > 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.
>> 
>> Just accepting an Iterator as the object to iterate trough in the
>> #foreach.
>
>Yes. Ok. Agreed.

How about Enumeration too, while you're at it.  I know it's deprecated but
it's still in a lot of APIs, including the Servlet API.



Re: Iterator introspection nonsense?

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Paulo Gaspar wrote:
> 
> Ok, just before going to sleep... I am wondering if a global cache would
> be so bad...
> 
> I will dream with it, play with it, make some performance tests and,
> after the home work done, I might agree with you...
> 
> ...but before that I am still wondering.
> =;o)

Its mostly a free world.  Enjoy.

geir
 
> Paulo
> 
> > -----Original Message-----
> > From: gmj@mta8.srv.hcvlny.cv.net [mailto:gmj@mta8.srv.hcvlny.cv.net]On
> > Behalf Of Geir Magnusson Jr.
> >
> > > > This icache stuff isn't global. You understand that, right?
> > > >
> > > > geir
> > >
> > > Why not?
> > > =:o)
> > >
> > > Paulo
> >
> > Reviewing the code should help with that one, but the basic idea is that
> > a template in AST form can be shared by multiple thread, each with a
> > different set of data in the contexts used byt those threads.  Since the
> > information that we are caching is therefore specific to the context, a
> > global cache makes no sense.  You could make some kind of thread local
> > storage, or a central caching resource that expires stuff etc, etc,, but
> > there was no need for that kind of machinery.  Yes, there is a global
> > introspector cache (which we don't actually use in Foreach now with the
> > latest changes), but once we do the expensive 'instanceof' (or before,
> > once we do the expensive and potentially blocking introspection), there
> > is no need to keep doing that.  Hence, the cache.  Results in quite a
> > performance improvement.
> >
> > geir

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

RE: Iterator introspection nonsense?

Posted by Paulo Gaspar <pa...@krankikom.de>.
Ok, just before going to sleep... I am wondering if a global cache would 
be so bad...

I will dream with it, play with it, make some performance tests and,
after the home work done, I might agree with you...

...but before that I am still wondering.
=;o)

Paulo

> -----Original Message-----
> From: gmj@mta8.srv.hcvlny.cv.net [mailto:gmj@mta8.srv.hcvlny.cv.net]On
> Behalf Of Geir Magnusson Jr.
>
> > > This icache stuff isn't global. You understand that, right?
> > >
> > > geir
> > 
> > Why not?
> > =:o)
> > 
> > Paulo
> 
> Reviewing the code should help with that one, but the basic idea is that
> a template in AST form can be shared by multiple thread, each with a
> different set of data in the contexts used byt those threads.  Since the
> information that we are caching is therefore specific to the context, a
> global cache makes no sense.  You could make some kind of thread local
> storage, or a central caching resource that expires stuff etc, etc,, but
> there was no need for that kind of machinery.  Yes, there is a global
> introspector cache (which we don't actually use in Foreach now with the
> latest changes), but once we do the expensive 'instanceof' (or before,
> once we do the expensive and potentially blocking introspection), there
> is no need to keep doing that.  Hence, the cache.  Results in quite a
> performance improvement.
> 
> geir


Re: Iterator introspection nonsense?

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Paulo Gaspar wrote:

> > This icache stuff isn't global. You understand that, right?
> >
> > geir
> 
> Why not?
> =:o)
> 
> Paulo

Reviewing the code should help with that one, but the basic idea is that
a template in AST form can be shared by multiple thread, each with a
different set of data in the contexts used byt those threads.  Since the
information that we are caching is therefore specific to the context, a
global cache makes no sense.  You could make some kind of thread local
storage, or a central caching resource that expires stuff etc, etc,, but
there was no need for that kind of machinery.  Yes, there is a global
introspector cache (which we don't actually use in Foreach now with the
latest changes), but once we do the expensive 'instanceof' (or before,
once we do the expensive and potentially blocking introspection), there
is no need to keep doing that.  Hence, the cache.  Results in quite a
performance improvement.

geir

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

RE: Iterator introspection nonsense?

Posted by Paulo Gaspar <pa...@krankikom.de>.
> -----Original Message-----
> From: gmj@mta8.srv.hcvlny.cv.net [mailto:gmj@mta8.srv.hcvlny.cv.net]On
> Behalf Of Geir Magnusson Jr.
>
>
> 
> This icache stuff isn't global. You understand that, right?
>  
> geir

Why not?
=:o)

Paulo

Re: Iterator introspection nonsense?

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Paulo Gaspar wrote:

> > > Now you cache what Iterator you got per Iterator object but it would be
> > > more efficient to cache it per listObject's class (listObject being the
> > > object to iterate trough in the #foreach).
> >
> > Nooo.... I don't think so.  I think I cache the class and type.
> >
> >             /*
> >              *  if we did figure it out, cache it
> >              */
> >
> >             if ( type != UNKNOWN )
> >             {
> >                 icd = new IntrospectionCacheData();
> >                 icd.thingy = new Integer( type );
> >                 icd.contextData = c;
> >                 context.icachePut( this, icd );
> >             }
> 
> Yes but "this", the Foreach object, is the key. It would be more efficient
> to have the iterated object's class being the key. You always get the same
> iterator a given iterated object class.

The Foreach object is the key - because we want to lookup by AST node -
that's the semantics of the node caching : for this node in the tree, we
store whatever is relevant. (What we store is different for each
node...)  The tree is immutable once it is parsed.

This icache stuff isn't global. You understand that, right?
 
geir

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

RE: Iterator introspection nonsense?

Posted by Paulo Gaspar <pa...@krankikom.de>.
Answer inline:

> -----Original Message-----
> From: gmj@mta8.srv.hcvlny.cv.net [mailto:gmj@mta8.srv.hcvlny.cv.net]On
> Behalf Of Geir Magnusson Jr.
> 
> ........
>
> > > sorry?
> > 
> > Now you cache what Iterator you got per Iterator object but it would be
> > more efficient to cache it per listObject's class (listObject being the
> > object to iterate trough in the #foreach).
> 
> Nooo.... I don't think so.  I think I cache the class and type.
> 
>             /*
>              *  if we did figure it out, cache it
>              */
> 
>             if ( type != UNKNOWN )
>             {
>                 icd = new IntrospectionCacheData();
>                 icd.thingy = new Integer( type );
>                 icd.contextData = c;
>                 context.icachePut( this, icd );
>             }

Yes but "this", the Foreach object, is the key. It would be more efficient
to have the iterated object's class being the key. You always get the same 
iterator a given iterated object class.
 
> > 
> > >...
> > >
> > > > That is a case where I would be better served with an Iterator as
> > > > argument for the #foreach, since an Iterator over a ResultSet is so
> > > > easy to implement.
> > > > =:o)
> > >
> > > I thought I had you above, but I seem to have lost you again : an
> > > Iterator is used in the foreach.  I am cleaning up getIterator() a
> > > little as per your's and Jeff's suggestions...
> > 
> > Yes, you get Iterator's from object arrays, Collection's and Map's ...
> > ...but what happens if the object to iterate trough in the #foreach is
> > _already_ an Iterator???
> 
> ah!  yep.  good one.

Thanks!
=:o)
 
 
> > > > Have fun,
> > > > Paulo
> > >
> > > Always do!
> > >
> > > geir
> > 
> > Doing my best to have it here too,
> > =:o)
> 
> *Thrilled* to hear it. :)
> 
> geir

=:o)
Paulo

Re: Iterator introspection nonsense?

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Paulo Gaspar wrote:
> 
> Answer inline:
> 
> > -----Original Message-----
> > From: gmj@mta4.srv.hcvlny.cv.net [mailto:gmj@mta4.srv.hcvlny.cv.net]On
> > Behalf Of Geir Magnusson Jr.
> >
> > ....
> >
> > > Just accepting an Iterator as the object to iterate trough in the
> > > #foreach.
> >
> > Yes. Ok. Agreed.
> >
> > ........
> >
> > > Yes, it works correctly but it would be more efficient to make the
> > > caching just using the listObject class as key.
> >
> > sorry?
> 
> Now you cache what Iterator you got per Iterator object but it would be
> more efficient to cache it per listObject's class (listObject being the
> object to iterate trough in the #foreach).

Nooo.... I don't think so.  I think I cache the class and type.

            /*
             *  if we did figure it out, cache it
             */

            if ( type != UNKNOWN )
            {
                icd = new IntrospectionCacheData();
                icd.thingy = new Integer( type );
                icd.contextData = c;
                context.icachePut( this, icd );
            }

> 
> >...
> >
> > > That is a case where I would be better served with an Iterator as
> > > argument for the #foreach, since an Iterator over a ResultSet is so
> > > easy to implement.
> > > =:o)
> >
> > I thought I had you above, but I seem to have lost you again : an
> > Iterator is used in the foreach.  I am cleaning up getIterator() a
> > little as per your's and Jeff's suggestions...
> 
> Yes, you get Iterator's from object arrays, Collection's and Map's ...
> ...but what happens if the object to iterate trough in the #foreach is
> _already_ an Iterator???

ah!  yep.  good one.


> > > Have fun,
> > > Paulo
> >
> > Always do!
> >
> > geir
> 
> Doing my best to have it here too,
> =:o)

*Thrilled* to hear it. :)

geir


-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

Re: Iterator introspection nonsense?

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
jeff wrote:
> 
> Rather than going overboard trying to second guess the userbase, have
> interfaces like these ever been considered?

No!  :)
 
> public interface VelocityMap
> {
>    void set(Object key, Object value);
>    Object get(Object key);
>    VelocityIterator keys();
> }
> 
> public interface VelocityIterator extends Iterator
> {
>     void reset();
> }


Actually, we talked about the VelIterator thing about 4 messages back in
this thread, mostly as exploration.

> These cover the two basic kinds of access used by Velocity.  VelocityMap is
> simpler to implement than java.util.Map and VelocityIterator contains the
> one piece of functionality lacking in the real Iterator.

I think supporting Map is easy in Vel, because we can just dig out an
Iterator.  If that's a pain to work with in ones own classes, a simple
Object[] is also supported, so there always is an easy way out.

> Of course, this still won't be as convenient for debugging as being able to
> toss Iterator and Enumeration directly into a #foreach, but it is simple,
> consistent and easy to understand.

Iterator is currently in there with a warning generated in the log and a
warning and discussion in th docs.  We can do the same with
Enumeration.  I wonder if that will be enough of a compromise : the
warning helps people figure out what went 'wrong' w/o <gasp> having to
read the documentation, and supporting Iterator and Enumeration *does*
make Velocity more useful for people who don't really have a choice.

geir

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

RE: Iterator introspection nonsense?

Posted by jeff <jl...@houseofdistraction.com>.
Rather than going overboard trying to second guess the userbase, have
interfaces like these ever been considered? 

public interface VelocityMap
{
   void set(Object key, Object value);
   Object get(Object key);
   VelocityIterator keys();
}

public interface VelocityIterator extends Iterator
{
    void reset();
}

These cover the two basic kinds of access used by Velocity.  VelocityMap is
simpler to implement than java.util.Map and VelocityIterator contains the
one piece of functionality lacking in the real Iterator.

Of course, this still won't be as convenient for debugging as being able to
toss Iterator and Enumeration directly into a #foreach, but it is simple,
consistent and easy to understand.



Re: Iterator introspection nonsense?

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Paulo Gaspar wrote:


> > >  2 - It is _major_ pain to implement a Map or a Collection, but
> > so easy to
> > >      implement an Iterator;
> > >  3 - Some Iterators (like over some performant forward only ResultSets)
> > >      naturally can not be reset. They are still damn useful.
> >
> > Right - I understand that the clueful of the world won't have a
> > problem.  I am worried about those that don't realize the problems they
> > will run into.  I am generally not a hand-holder, but we are trying to
> > make a simple, clean tool targeted towards simplicity for the designer,
> > a different skill set than the programmer.  Yes, a programmer has to be
> > involved, but I am wondering how long it will be before someone puts
> > together a scriptable servlet where a designer can toss premade beans
> > into the context via external script/xml, etc, etc, etc...
> 
> Well, from where I come they say:
>  - Don't dress like a wolf if you aren't one.

Something must be lost in the translation :)

> 
> > We can certainly support it.  I would almost be rude not to.  Maybe the
> > way to go is to holler and scream into the log when we run across a
> > naked Iterator....
> 
> Yeah! On warning level!

done.
 
> > > Maybe I really need plugable introspection.
> >
> > You certainly need something...
> 
> ...and I hope to get it here, since I don't get scared looking at Velocity
> source code. (But I sure do when looking at WM's.)

Why are WM's so scary?

geir
 
> 
> And keep having fun,
> =:o)
> 
> Paulo

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

RE: Iterator introspection nonsense?

Posted by Paulo Gaspar <pa...@krankikom.de>.
Inline answer:

> -----Original Message-----
> From: gmj@mta4.srv.hcvlny.cv.net [mailto:gmj@mta4.srv.hcvlny.cv.net]On
> Behalf Of Geir Magnusson Jr.
> 
> Paulo Gaspar wrote:
> > 
> > I still like it. It solves big problems like mine -> loads of 
> data. It can
> > save a lot of resources.
> 
> There still may be a solution that you as a talented Java programmer
> could employ w/o doing something that I  am *sure* will bewilder and
> confuse people.
>  
> Or not.  Understand that I understand your issue.
> 
> > Look:
> >  1 - The programmer implementing the Iterator and placing it in 
> the Context,
> >      MUST know what an Iterator is.
> 
> I am not 100% sure of that. :)

Man! Do we have to baby-sit those? 
At least we would give them an opportunity to learn!
=;o)

> 
> >      And he can implement a reset method
> >      usable in the template anyway - it will still be an Iterator
> >      implementation. What you point out is an Application issue, not a
> >      library one;
> >      =:o)
> 
> 
> The point is not that there is some taboo about the iterator itself -
> the question is how can we avoid the problem noted, that if it's an
> Iterator placed into the context, someone is going to run aground.
> 
> <self-loathing>
> Would it be worth it to consider defining an interface like
> 
> public interface VelIterator extends Iterator
> {
> 	public boolean reset();
> }
> 
> so you can wrap your Iterator, but then there are no surprises re
> reset()?  Foreach or any other looping contruct that may come down the
> pike will call it after use, and if you are doing something 'special',
> like working with a foward only RS, then so be it.  You are at least
> consciously making reset() do nothing...
> 
> </self-loathing>

Playing Devil Advocate again!

But then you break one of (your) golden rules by placing in the framework 
something that should belong to the application level.

(Even if a VelIteratorSimpleWraper class makes it simple to use standard 
Iterators.)


> >  2 - It is _major_ pain to implement a Map or a Collection, but 
> so easy to
> >      implement an Iterator;
> >  3 - Some Iterators (like over some performant forward only ResultSets)
> >      naturally can not be reset. They are still damn useful.
> 
> Right - I understand that the clueful of the world won't have a
> problem.  I am worried about those that don't realize the problems they
> will run into.  I am generally not a hand-holder, but we are trying to
> make a simple, clean tool targeted towards simplicity for the designer,
> a different skill set than the programmer.  Yes, a programmer has to be
> involved, but I am wondering how long it will be before someone puts
> together a scriptable servlet where a designer can toss premade beans
> into the context via external script/xml, etc, etc, etc...

Well, from where I come they say:
 - Don't dress like a wolf if you aren't one.

 
> We can certainly support it.  I would almost be rude not to.  Maybe the
> way to go is to holler and scream into the log when we run across a
> naked Iterator....

Yeah! On warning level!

> > Maybe I really need plugable introspection.
> 
> You certainly need something...

...and I hope to get it here, since I don't get scared looking at Velocity
source code. (But I sure do when looking at WM's.)

 
And keep having fun,
=:o)

Paulo

Re: Iterator introspection nonsense?

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Paulo Gaspar wrote:
> 
> I still like it. It solves big problems like mine -> loads of data. It can
> save a lot of resources.

There still may be a solution that you as a talented Java programmer
could employ w/o doing something that I  am *sure* will bewilder and
confuse people.
 
Or not.  Understand that I understand your issue.

> Look:
>  1 - The programmer implementing the Iterator and placing it in the Context,
>      MUST know what an Iterator is.

I am not 100% sure of that. :)

>      And he can implement a reset method
>      usable in the template anyway - it will still be an Iterator
>      implementation. What you point out is an Application issue, not a
>      library one;
>      =:o)


The point is not that there is some taboo about the iterator itself -
the question is how can we avoid the problem noted, that if it's an
Iterator placed into the context, someone is going to run aground.

<self-loathing>
Would it be worth it to consider defining an interface like

public interface VelIterator extends Iterator
{
	public boolean reset();
}

so you can wrap your Iterator, but then there are no surprises re
reset()?  Foreach or any other looping contruct that may come down the
pike will call it after use, and if you are doing something 'special',
like working with a foward only RS, then so be it.  You are at least
consciously making reset() do nothing...

</self-loathing>



>  2 - It is _major_ pain to implement a Map or a Collection, but so easy to
>      implement an Iterator;
>  3 - Some Iterators (like over some performant forward only ResultSets)
>      naturally can not be reset. They are still damn useful.

Right - I understand that the clueful of the world won't have a
problem.  I am worried about those that don't realize the problems they
will run into.  I am generally not a hand-holder, but we are trying to
make a simple, clean tool targeted towards simplicity for the designer,
a different skill set than the programmer.  Yes, a programmer has to be
involved, but I am wondering how long it will be before someone puts
together a scriptable servlet where a designer can toss premade beans
into the context via external script/xml, etc, etc, etc...

We can certainly support it.  I would almost be rude not to.  Maybe the
way to go is to holler and scream into the log when we run across a
naked Iterator....

> Maybe I really need plugable introspection.

You certainly need something...

> =:o)
> 
> Have fun,
> Paulo

Always do.

geir

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

RE: Iterator introspection nonsense?

Posted by Paulo Gaspar <pa...@krankikom.de>.
I still like it. It solves big problems like mine -> loads of data. It can
save a lot of resources.

Look:
 1 - The programmer implementing the Iterator and placing it in the Context,
     MUST know what an Iterator is. And he can implement a reset method
     usable in the template anyway - it will still be an Iterator
     implementation. What you point out is an Application issue, not a
     library one;
     =:o)
 2 - It is _major_ pain to implement a Map or a Collection, but so easy to
     implement an Iterator;
 3 - Some Iterators (like over some performant forward only ResultSets)
     naturally can not be reset. They are still damn useful.

Maybe I really need plugable introspection.
=:o)

Have fun,
Paulo

> -----Original Message-----
> From: gmj@mta8.srv.hcvlny.cv.net [mailto:gmj@mta8.srv.hcvlny.cv.net]On
> Behalf Of Geir Magnusson Jr.
>
> Paulo Gaspar wrote:
>
> > Yes, you get Iterator's from object arrays, Collection's and Map's ...
> > ...but what happens if the object to iterate trough in the #foreach is
> > _already_ an Iterator???
>
> Thinking about this, allowing an Iterator will lead to trouble -
>
> code  (vec is a Vector) :
>
> context.put("iterator", vec.iterator() );
>
> template :
>
> #foreach($i in $iterator)
>   $i
> #end
>
> #foreach($j in $iterator)
>   $j
> #end
>
>
> Will result in a surprise...  there's no way to 'reset' the Iterator, so
> the second foreach loop wouldn't output anything.
>
> This is a problem.
>
> I think we shouldn't allow using Iterator's directly in the context, as
> this will simply lead to trouble - no matter how well documented, this
> kind of statefulness w.o. the ability to reset will lead to problems for
> people.
>
> I can put a message in the log if we come across an Iterator with a
> warning, because it *is* sort of a natural thing to do, but we should
> probably try to dissuade people from using this.
>
> Comments, anyone?
>
> geir
>


Re: Iterator introspection nonsense?

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Paulo Gaspar wrote:

> Yes, you get Iterator's from object arrays, Collection's and Map's ...
> ...but what happens if the object to iterate trough in the #foreach is
> _already_ an Iterator???

Thinking about this, allowing an Iterator will lead to trouble - 

code  (vec is a Vector) :

context.put("iterator", vec.iterator() );

template :

#foreach($i in $iterator)
  $i
#end

#foreach($j in $iterator)
  $j
#end


Will result in a surprise...  there's no way to 'reset' the Iterator, so
the second foreach loop wouldn't output anything.

This is a problem.

I think we shouldn't allow using Iterator's directly in the context, as
this will simply lead to trouble - no matter how well documented, this
kind of statefulness w.o. the ability to reset will lead to problems for
people.

I can put a message in the log if we come across an Iterator with a
warning, because it *is* sort of a natural thing to do, but we should
probably try to dissuade people from using this.

Comments, anyone?

geir

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

RE: Iterator introspection nonsense?

Posted by Paulo Gaspar <pa...@krankikom.de>.
Answer inline:

> -----Original Message-----
> From: gmj@mta4.srv.hcvlny.cv.net [mailto:gmj@mta4.srv.hcvlny.cv.net]On
> Behalf Of Geir Magnusson Jr.
> 
> ....
>
> > Just accepting an Iterator as the object to iterate trough in the
> > #foreach.
> 
> Yes. Ok. Agreed.
>  
> ........
>
> > Yes, it works correctly but it would be more efficient to make the
> > caching just using the listObject class as key.
> 
> sorry?

Now you cache what Iterator you got per Iterator object but it would be
more efficient to cache it per listObject's class (listObject being the 
object to iterate trough in the #foreach).

>...
>
> > That is a case where I would be better served with an Iterator as
> > argument for the #foreach, since an Iterator over a ResultSet is so
> > easy to implement.
> > =:o)
> 
> I thought I had you above, but I seem to have lost you again : an
> Iterator is used in the foreach.  I am cleaning up getIterator() a
> little as per your's and Jeff's suggestions...

Yes, you get Iterator's from object arrays, Collection's and Map's ...
...but what happens if the object to iterate trough in the #foreach is
_already_ an Iterator???

 
> > Have fun,
> > Paulo
> 
> Always do!
> 
> geir

Doing my best to have it here too,
=:o)

Paulo

Re: Iterator introspection nonsense?

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Paulo Gaspar wrote:
> 
> Answer inline:
> 
> > -----Original Message-----
> > From: gmj@mta4.srv.hcvlny.cv.net [mailto:gmj@mta4.srv.hcvlny.cv.net]On
> > Behalf Of Geir Magnusson Jr.
> >
> > 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.
> 
> Just accepting an Iterator as the object to iterate trough in the
> #foreach.

Yes. Ok. Agreed.
 
> > ........
> > >   - 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.
> 
> Yes, it works correctly but it would be more efficient to make the
> caching just using the listObject class as key.

sorry?

> > > 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?
> 
> Means that I will "give it a whack" as you also suggested.
> =:o)
> 
> > 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
> 
> Not using yet. Just taking a look at the source.
> 
> I want to use this to make an export tool that will have to iterate
> over big JDBC ResultSet's and I was taking a look at how to do it.

Oh no!

;->
 
> That is a case where I would be better served with an Iterator as
> argument for the #foreach, since an Iterator over a ResultSet is so
> easy to implement.
> =:o)

I thought I had you above, but I seem to have lost you again : an
Iterator is used in the foreach.  I am cleaning up getIterator() a
little as per your's and Jeff's suggestions...

> 
> Have fun,
> Paulo

Always do!

geir

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

RE: Iterator introspection nonsense?

Posted by Paulo Gaspar <pa...@krankikom.de>.
Answer inline:

> -----Original Message-----
> From: gmj@mta4.srv.hcvlny.cv.net [mailto:gmj@mta4.srv.hcvlny.cv.net]On
> Behalf Of Geir Magnusson Jr.
> 
> 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.

Just accepting an Iterator as the object to iterate trough in the
#foreach.

> ........
> >   - 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.

Yes, it works correctly but it would be more efficient to make the 
caching just using the listObject class as key.


> > 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?
 
Means that I will "give it a whack" as you also suggested.
=:o)

> 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

Not using yet. Just taking a look at the source.

I want to use this to make an export tool that will have to iterate 
over big JDBC ResultSet's and I was taking a look at how to do it.

That is a case where I would be better served with an Iterator as 
argument for the #foreach, since an Iterator over a ResultSet is so 
easy to implement. 
=:o)

Have fun,
Paulo

Re: Iterator introspection nonsense?

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
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.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity