You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by nb...@apache.org on 2004/05/04 05:32:22 UTC
cvs commit: jakarta-velocity-tools/src/java/org/apache/velocity/tools/generic Alternator.java AlternatorTool.java
nbubna 2004/05/03 20:32:22
Added: src/java/org/apache/velocity/tools/generic Alternator.java
AlternatorTool.java
Log:
initial revision of convenient 'alternator' support
Revision Changes Path
1.1 jakarta-velocity-tools/src/java/org/apache/velocity/tools/generic/Alternator.java
Index: Alternator.java
===================================================================
/*
* Copyright 2004 The Apache Software Foundation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.velocity.tools.generic;
import java.util.List;
/**
* Utility class for easily alternating over values in a list.
*
* <p><b>Example Use:</b>
* <pre>
* java...
* List myColors = new ArrayList();
* myColors.add("red");
* myColors.add("blue");
* context.put("color", new Alternator(myColors));
* List myStyles = new ArrayList();
* myColors.add("hip");
* myColors.add("fly");
* myColors.add("groovy");
* // use auto alternation
* context.put("style", new Alternator(true, myStyles));
*
* template...
* #foreach( $foo in [1..5] )
* $foo is $color.next and $style
* #end
*
* output...
* 1 is red and hip
* 2 is blue and fly
* 3 is red and groovy
* 4 is blue and hip
* 5 is red and fly
* </pre></p>
*
* @version $Revision: 1.1 $ $Date: 2004/05/04 03:32:22 $
*/
public class Alternator
{
private List list;
private int index = 0;
private boolean auto = false;
/**
* Creates a new Alternator for the specified list.
*/
public Alternator(List list)
{
this.list = list;
}
/**
* Creates a new Alternator for the specified list with the specified
* automatic shifting preference. See {@link #setAuto(boolean auto)}.
*/
public Alternator(boolean auto, List list)
{
this.auto = auto;
this.list = list;
}
/**
* Returns true if this Alternator shifts the list index automatically
* after a call to toString().
*/
public boolean isAuto()
{
return auto;
}
/**
* If set to true, the list index will shift automatically after a
* call to toString().
*/
public void setAuto(boolean auto)
{
this.auto = auto;
}
/**
* Manually shifts the list index. If it reaches the end of the list,
* it will start over again at zero.
*/
public void shift()
{
index = (index + 1) % list.size();
}
/**
* Returns the current item without shifting the list index.
*/
public Object getCurrent()
{
return list.get(index);
}
/**
* Returns the current item before shifting the list index.
*/
public Object getNext()
{
Object o = getCurrent();
shift();
return o;
}
/**
* Returns a string representation of the current item or
* <code>null</code> if the current item is null. Also,
* if <i>auto</i> is true, this will shift after returning
* the current item.
*/
public String toString()
{
Object o = list.get(index);
if (auto)
{
shift();
}
if (o == null)
{
return null;
}
return o.toString();
}
}
1.1 jakarta-velocity-tools/src/java/org/apache/velocity/tools/generic/AlternatorTool.java
Index: AlternatorTool.java
===================================================================
/*
* Copyright 2004 The Apache Software Foundation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.velocity.tools.generic;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
/**
* Simple tool to provide easy in-template instantiation of
* {@link Alternator}s from varying "list" types.
*
* <p><b>Example Use:</b>
* <pre>
* toolbox.xml...
* <tool>
* <key>alternator</key>
* <scope>application</scope>
* <class>org.apache.velocity.tools.generic.AlternatorTool</class>
* </tool>
*
* template...
* #set( $color = $alternator.make('red', 'blue') )
* #set( $style = $alternator.make(true, ['hip','fly','groovy']) )
* #foreach( $i in [1..5] )
* $i is $color.next and $style
* #end
*
* output...
* 1 is red and hip
* 2 is blue and fly
* 3 is red and groovy
* 4 is blue and hip
* 5 is red and fly
* </pre></p>
*
* @version $Revision: 1.1 $ $Date: 2004/05/04 03:32:22 $
*/
public class AlternatorTool
{
public AlternatorTool() {}
/**
* Make a non-automatic {@link Alternator} from a List.
*/
public Alternator make(List list)
{
return make(false, list);
}
/**
* Make an {@link Alternator} from a List.
*/
public Alternator make(boolean auto, List list)
{
if (list == null)
{
return null;
}
return new Alternator(auto, list);
}
/**
* Make a non-automatic {@link Alternator} from the values
* in the specified collection.
*/
public Alternator make(Collection collection)
{
return make(false, collection);
}
/**
* Make an {@link Alternator} from the values
* in the specified collection.
*/
public Alternator make(boolean auto, Collection collection)
{
if (collection == null)
{
return null;
}
return make(auto, new ArrayList(collection));
}
/**
* Make a non-automatic {@link Alternator} from an object array.
*/
public Alternator make(Object[] array)
{
return make(false, array);
}
/**
* Make an {@link Alternator} from an object array.
*/
public Alternator make(boolean auto, Object[] array)
{
if (array == null)
{
return null;
}
return make(auto, Arrays.asList(array));
}
/**
* Make a non-automatic {@link Alternator} from a list containing the two
* specified objects.
*/
public Alternator make(Object o1, Object o2)
{
return make(false, o1, o2);
}
/**
* Make an {@link Alternator} from a list containing the two
* specified objects.
*/
public Alternator make(boolean auto, Object o1, Object o2)
{
List list = new ArrayList();
list.add(o1);
list.add(o2);
return make(auto, list);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
Re: Alternator and AlternatorTool
Posted by Nathan Bubna <na...@esha.com>.
Daniel Rall said:
> Nathan Bubna wrote:
...
> >>More comments inline...
> >>nbubna@apache.org wrote:
> >>...
> >>This class is an ideal candidate for JUnit tests.
>
> I was looked very briefly for a way to write a JUnit test, but didn't notice
a
> framework for doing so in the Velocity Tools package.
<confession type="guilty">
I've yet to use JUnit or any other automated code testing.
</confession>
this is just one of those things that i've always felt i should do but was
never motivated enough to get around to doing. if anyone more familiar with
this stuff is willing to help, that'd be great.
> >>> public class Alternator
> >>> {
> >>> private List list;
> >>> private int index = 0;
> >>> private boolean auto = false;
> >>>
> >>> /**
> >>> * Creates a new Alternator for the specified list.
> >>> */
> >>> public Alternator(List list)
> >>> {
> >>> this.list = list;
> >>> }
> >>
> >>Why not default to auto? This will be the de facto use case for this
tool.
> >
> >
> > i was trying to follow the principle of least surprise and felt that have
> > toString() work like that be default might be pretty surprising. but i
don't
> > feel strongly about this. if anyone else chimes in on this, feel free to
> > change it.
>
> I see what you mean. Still, I feel pretty strongly that "auto" is the
common
> use case, and that the default behavior should support that use case.
that's good enough for me. :)
> Regarding
> your point about side-effects, I see documentation as the answer. I'll add
the
> JavaDoc, but is there somewhere else I should document that as well?
not at the moment, javadoc is all i've got for these classes so far. i did
fix up a few places in the javadoc that you missed though. :)
> >>All the ctors might be best off throwing IllegalArgumentException when
> >>passed a null list.
> >
> > ok. but only as long as the AlternatorTool catches them or
> > otherwise ensures they don't happen during template rendering.
> > i generally prefer tools return nulls rather than throw exceptions.
>
> For this case, me too. I was thinking that AlternatorTool would retain its
null
> checking behavior, but Alternator itself would acquire the exception
raising. I
> see this as better support for systems which don't use the toolbox concept
> (e.g. side-stepping AlternatorTool all together).
sounds good to me. go for it.
> ...
> >>> /**
> >>> * Returns the current item without shifting the list index.
> >>> */
> >>> public Object getCurrent()
> >>> {
> >>> return list.get(index);
> >>> }
> >>>
> >>> /**
> >>> * Returns the current item before shifting the list index.
> >>> */
> >>> public Object getNext()
> >>> {
> >>> Object o = getCurrent();
> >>> shift();
> >>> return o;
> >>> }
> >>
> >>getNext() is a really confusing name for this method, and doesn't provide
> >>behavior orthogonal to that of getCurrent(). What it's doing seems to be
> >>more along the lines of "get current then shift", which would be more
> >>appropriately be named along the lines of "getThenShift()".
> >
> > getNext() is so named to parallel the Iterator paradigm of "next",
> > but in a way that is more VTL friendly (e.g. $color.next instead of
> > $color.next() ). i think it is a common enough paradigm to not
> > confuse people for too long. and i don't like the idea of
> > $color.thenShift i think that would be more confusing.
>
> Yeah, $color.thenShift sucks. It's somewhat confusing that what's
internally
> the "current" state is externally the "next" state. *shrug*
yeah, i guess the difference between implementation and interface is
confusing.
> When you explain it like that, it makes more sense. Perhaps
> more JavaDoc to differentiate between getCurrent() and getNext()
> is what I'm looking for here. Dunno..
hey, i'm always a fan of having more javadoc. :)
> > getCurrent() exists only to provide a means for retrieving the current
object
> > without shifting. this isn't very important for non-auto alternators, but
i
> > figure it may come in handy with auto-alternators when both toString() and
> > getNext() would shift.
>
> Yeah.
>
> >>> public class AlternatorTool
> >>> {
> >>>
> >>> public AlternatorTool() {}
> >>
> >>Any reason for this ctor to exist?
> >
> > no practical one.
>
> The only reason I could come up with is "just in case someone wants to
> Class.forName().newInstance()-it", which didn't seem to compelling, but you
> never know. Think I should remove it?
...
i really don't care either way.
Nathan Bubna
nathan@esha.com
---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
Re: Alternator and AlternatorTool
Posted by Daniel Rall <dl...@collab.net>.
Nathan Bubna wrote:
...
> yeah, i guess that make sense. thanks!
:)
>>More comments inline...
>>nbubna@apache.org wrote:
>>...
>>
>>This class is an ideal candidate for JUnit tests.
I was looked very briefly for a way to write a JUnit test, but didn't notice a
framework for doing so in the Velocity Tools package.
>>> public class Alternator
>>> {
>>> private List list;
>>> private int index = 0;
>>> private boolean auto = false;
>>>
>>> /**
>>> * Creates a new Alternator for the specified list.
>>> */
>>> public Alternator(List list)
>>> {
>>> this.list = list;
>>> }
>>
>>Why not default to auto? This will be the de facto use case for this tool.
>
>
> i was trying to follow the principle of least surprise and felt that have
> toString() work like that be default might be pretty surprising. but i don't
> feel strongly about this. if anyone else chimes in on this, feel free to
> change it.
I see what you mean. Still, I feel pretty strongly that "auto" is the common
use case, and that the default behavior should support that use case. Regarding
your point about side-effects, I see documentation as the answer. I'll add the
JavaDoc, but is there somewhere else I should document that as well?
>>All the ctors might be best off throwing IllegalArgumentException when
>>passed a null list.
>
> ok. but only as long as the AlternatorTool catches them or otherwise ensures
> they don't happen during template rendering. i generally prefer tools return
> nulls rather than throw exceptions.
For this case, me too. I was thinking that AlternatorTool would retain its null
checking behavior, but Alternator itself would acquire the exception raising. I
see this as better support for systems which don't use the toolbox concept (e.g.
side-stepping AlternatorTool all together).
...
>>> /**
>>> * Returns the current item without shifting the list index.
>>> */
>>> public Object getCurrent()
>>> {
>>> return list.get(index);
>>> }
>>>
>>> /**
>>> * Returns the current item before shifting the list index.
>>> */
>>> public Object getNext()
>>> {
>>> Object o = getCurrent();
>>> shift();
>>> return o;
>>> }
>>
>>getNext() is a really confusing name for this method, and doesn't provide
>>behavior orthogonal to that of getCurrent(). What it's doing seems to be
>>more along the lines of "get current then shift", which would be more
>>appropriately be named along the lines of "getThenShift()".
>
> getNext() is so named to parallel the Iterator paradigm of "next", but in a
> way that is more VTL friendly (e.g. $color.next instead of $color.next() ). i
> think it is a common enough paradigm to not confuse people for too long. and
> i don't like the idea of $color.thenShift i think that would be more
> confusing.
Yeah, $color.thenShift sucks. It's somewhat confusing that what's internally
the "current" state is externally the "next" state. *shrug* When you explain
it like that, it makes more sense. Perhaps more JavaDoc to differentiate
between getCurrent() and getNext() is what I'm looking for here. Dunno..
> getCurrent() exists only to provide a means for retrieving the current object
> without shifting. this isn't very important for non-auto alternators, but i
> figure it may come in handy with auto-alternators when both toString() and
> getNext() would shift.
Yeah.
>>> public class AlternatorTool
>>> {
>>>
>>> public AlternatorTool() {}
>>
>>Any reason for this ctor to exist?
>
> no practical one.
The only reason I could come up with is "just in case someone wants to
Class.forName().newInstance()-it", which didn't seem to compelling, but you
never know. Think I should remove it?
>>...
>>> /**
>>> * Make a non-automatic {@link Alternator} from the values
>>> * in the specified collection.
>>> */
>>> public Alternator make(Collection collection)
>>> {
>>> return make(false, collection);
>>> }
>>>
>>> /**
>>> * Make an {@link Alternator} from the values
>>> * in the specified collection.
>>> */
>>> public Alternator make(boolean auto, Collection collection)
>>> {
>>> if (collection == null)
>>> {
>>> return null;
>>> }
>>> return make(auto, new ArrayList(collection));
>>> }
>>
>>Use of java.util.Collection implies that order is not important. For the
>>Alternator class, order IS important. If you're really set on retaining this
>>the API, I strongly recommend that you document that the list will be created in
>>order returned by the Collection's Iterator. Personally, I think this sort
>>of behavior is something which should be left up to the caller.
>
> yeah, the Collection support was a last minute thought. i just figured since
> #foreach can handle Maps as well as Lists and Object[]s, we might as well
> support Maps with Alternator. this was just the simplest way. i'm not too
> attached to it if any thinks it should go.
Okay, done. Worst case, this means an extra "new ArrayList(collection)" for the
caller when collection isn't an instance of List.
---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
Re: cvs commit:
jakarta-velocity-tools/src/java/org/apache/velocity/tools/generic
Alternator.java AlternatorTool.java
Posted by Nathan Bubna <na...@esha.com>.
Daniel Rall said:
> Nathan, your Alternator class is a really great implementation of this sort
of
> tool. As the list will usually be static, I've modified the implementation
to
> use an Object[] array internally, and the AlternatorTool factory class to
> leverage that.
yeah, i guess that make sense. thanks!
> More comments inline...
> nbubna@apache.org wrote:
> ...
>
> This class is an ideal candidate for JUnit tests.
>
> > public class Alternator
> > {
> > private List list;
> > private int index = 0;
> > private boolean auto = false;
> >
> > /**
> > * Creates a new Alternator for the specified list.
> > */
> > public Alternator(List list)
> > {
> > this.list = list;
> > }
>
> Why not default to auto? This will be the de facto use case for this tool.
i was trying to follow the principle of least surprise and felt that have
toString() work like that be default might be pretty surprising. but i don't
feel strongly about this. if anyone else chimes in on this, feel free to
change it.
> All the ctors might be best off throwing IllegalArgumentException when
passed a
> null list.
ok. but only as long as the AlternatorTool catches them or otherwise ensures
they don't happen during template rendering. i generally prefer tools return
nulls rather than throw exceptions.
> ...
> > /**
> > * Manually shifts the list index. If it reaches the end of the
list,
> > * it will start over again at zero.
> > */
> > public void shift()
> > {
> > index = (index + 1) % list.size();
> > }
>
> shiftIndex() might be more descriptive. I like short names, so don't feel
> strongly about changing this.
i still prefer the short name.
> > /**
> > * Returns the current item without shifting the list index.
> > */
> > public Object getCurrent()
> > {
> > return list.get(index);
> > }
> >
> > /**
> > * Returns the current item before shifting the list index.
> > */
> > public Object getNext()
> > {
> > Object o = getCurrent();
> > shift();
> > return o;
> > }
>
> getNext() is a really confusing name for this method, and doesn't provide
> behavior orthogonal to that of getCurrent(). What it's doing seems to be
more
> along the lines of "get current then shift", which would be more
appropriately
> be named along the lines of "getThenShift()".
getNext() is so named to parallel the Iterator paradigm of "next", but in a
way that is more VTL friendly (e.g. $color.next instead of $color.next() ). i
think it is a common enough paradigm to not confuse people for too long. and
i don't like the idea of $color.thenShift i think that would be more
confusing.
getCurrent() exists only to provide a means for retrieving the current object
without shifting. this isn't very important for non-auto alternators, but i
figure it may come in handy with auto-alternators when both toString() and
getNext() would shift.
> ...
> > public class AlternatorTool
> > {
> >
> > public AlternatorTool() {}
>
> Any reason for this ctor to exist?
no practical one.
> ...
> > /**
> > * Make a non-automatic {@link Alternator} from the values
> > * in the specified collection.
> > */
> > public Alternator make(Collection collection)
> > {
> > return make(false, collection);
> > }
> >
> > /**
> > * Make an {@link Alternator} from the values
> > * in the specified collection.
> > */
> > public Alternator make(boolean auto, Collection collection)
> > {
> > if (collection == null)
> > {
> > return null;
> > }
> > return make(auto, new ArrayList(collection));
> > }
>
> Use of java.util.Collection implies that order is not important. For the
> Alternator class, order IS important. If you're really set on retaining
this
> API, I strongly recommend that you document that the list will be created in
the
> order returned by the Collection's Iterator. Personally, I think this sort
of
> behavior is something which should be left up to the caller.
...
yeah, the Collection support was a last minute thought. i just figured since
#foreach can handle Maps as well as Lists and Object[]s, we might as well
support Maps with Alternator. this was just the simplest way. i'm not too
attached to it if any thinks it should go.
Nathan Bubna
nathan@esha.com
---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-velocity-tools/src/java/org/apache/velocity/tools/generic
Alternator.java AlternatorTool.java
Posted by Daniel Rall <dl...@collab.net>.
Nathan, your Alternator class is a really great implementation of this sort of
tool. As the list will usually be static, I've modified the implementation to
use an Object[] array internally, and the AlternatorTool factory class to
leverage that. More comments inline...
nbubna@apache.org wrote:
...
This class is an ideal candidate for JUnit tests.
> public class Alternator
> {
> private List list;
> private int index = 0;
> private boolean auto = false;
>
> /**
> * Creates a new Alternator for the specified list.
> */
> public Alternator(List list)
> {
> this.list = list;
> }
Why not default to auto? This will be the de facto use case for this tool.
All the ctors might be best off throwing IllegalArgumentException when passed a
null list.
...
> /**
> * Manually shifts the list index. If it reaches the end of the list,
> * it will start over again at zero.
> */
> public void shift()
> {
> index = (index + 1) % list.size();
> }
shiftIndex() might be more descriptive. I like short names, so don't feel
strongly about changing this.
> /**
> * Returns the current item without shifting the list index.
> */
> public Object getCurrent()
> {
> return list.get(index);
> }
>
> /**
> * Returns the current item before shifting the list index.
> */
> public Object getNext()
> {
> Object o = getCurrent();
> shift();
> return o;
> }
getNext() is a really confusing name for this method, and doesn't provide
behavior orthogonal to that of getCurrent(). What it's doing seems to be more
along the lines of "get current then shift", which would be more appropriately
be named along the lines of "getThenShift()".
...
> public class AlternatorTool
> {
>
> public AlternatorTool() {}
Any reason for this ctor to exist?
...
> /**
> * Make a non-automatic {@link Alternator} from the values
> * in the specified collection.
> */
> public Alternator make(Collection collection)
> {
> return make(false, collection);
> }
>
> /**
> * Make an {@link Alternator} from the values
> * in the specified collection.
> */
> public Alternator make(boolean auto, Collection collection)
> {
> if (collection == null)
> {
> return null;
> }
> return make(auto, new ArrayList(collection));
> }
Use of java.util.Collection implies that order is not important. For the
Alternator class, order IS important. If you're really set on retaining this
API, I strongly recommend that you document that the list will be created in the
order returned by the Collection's Iterator. Personally, I think this sort of
behavior is something which should be left up to the caller.
> /**
> * Make a non-automatic {@link Alternator} from an object array.
> */
> public Alternator make(Object[] array)
> {
> return make(false, array);
> }
>
> /**
> * Make an {@link Alternator} from an object array.
> */
> public Alternator make(boolean auto, Object[] array)
> {
> if (array == null)
> {
> return null;
> }
> return make(auto, Arrays.asList(array));
> }
...
---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org