Hi Jordan, >> * 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.
the LinkedHashSet[1] will preserve the order like the List does, moreover will keep the unicity of its elements. > If you open a ticket on this I'll make the changes. sure, will do ASAP! :) Best, -Simo [1] http://docs.oracle.com/javase/6/docs/api/java/util/LinkedHashSet.html http://people.apache.org/~simonetripodi/ http://simonetripodi.livejournal.com/ http://twitter.com/simonetripodi http://www.99soft.org/ On Mon, Mar 11, 2013 at 9:59 PM, Jordan Zimmerman <jor...@jordanzimmerman.com> wrote: >> * 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/ >
