> * calling it ListBuilder is a little confusing, because the name > reminds me a collections general purpose class, I'd rename it to > LifecycleAnnotationsListBuilder; That's probably a better name. Ideally, I'd just use Guava but I didn't want to introduce another dependency.
> * the adapted List of annotation should be IMHO a LinkedHashSet, > otherwise users could specify the same type twice in the list and > drive the lifecycle engine to invoke twice the same method in > different phases - IIUC this is not the desired behaviour, or is it? It's a List because it needs to be ordered. I hadn't thought about duplicates. Maybe the right thing to do is to turn this into a "first class" object instead of just a helper. Like you say in your third point, have LifeCycleModule only take LifecycleAnnotationsList. LifecycleAnnotationsListBuilder would build an ordered, unique list. If you open a ticket on this I'll make the changes. -JZ On Mar 11, 2013, at 1:18 PM, Simone Tripodi <simonetrip...@apache.org> wrote: > Hi Jordan/all, > > while reviewing the incredible work that Jordan just checked-in in > [lifecycle] module, I got interested by ListBuilder class, I have few > questions/observations: > > * calling it ListBuilder is a little confusing, because the name > reminds me a collections general purpose class, I'd rename it to > LifecycleAnnotationsListBuilder; > > * the adapted List of annotation should be IMHO a LinkedHashSet, > otherwise users could specify the same type twice in the list and > drive the lifecycle engine to invoke twice the same method in > different phases - IIUC this is not the desired behaviour, or is it? > > * according to previous point, > org.apache.onami.lifecycle.core.LifeCycleModule should be modified in > order to accept a more appropriate data structure. > > WDYT? > TIA, all the best, > -Simo > > http://people.apache.org/~simonetripodi/ > http://simonetripodi.livejournal.com/ > http://twitter.com/simonetripodi > http://www.99soft.org/
