[EMAIL PROTECTED] 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 [EMAIL PROTECTED] Alternator} from the values * in the specified collection.
*/
public Alternator make(Collection collection)
{
return make(false, collection);
}
/**
* Make an [EMAIL PROTECTED] 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 [EMAIL PROTECTED] Alternator} from an object array.
*/
public Alternator make(Object[] array)
{
return make(false, array);
}
/**
* Make an [EMAIL PROTECTED] 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]