Thanks Jonas, would you submit a patch for that? TIA!
-Simo http://people.apache.org/~simonetripodi/ http://twitter.com/simonetripodi On Thu, Jun 20, 2013 at 11:09 PM, Jonas Sprenger <sprenger...@gmail.com> wrote: > Hi guys, > > so we talk about an enum 'PROCESSING' providing at least 3 states? > > - CONTINUE > - COMPLETE > - ABORTED > > Ticket CHAIN-98 <https://issues.apache.org/jira/browse/CHAIN-98> has been > created to cover this task. > > regards, > Jonas > > 2013/6/20 Simone Tripodi <simonetrip...@apache.org> > >> Hallo Bene, >> >> > * JavaDoc of Command says that commands have to be designed in a >> > thread-safe manner, yet all the base implementations are not thread >> safe! I >> > have created CHAIN-96 [1] for this, because this definitely has to be >> fixed >> > before 2.0 >> >> +1 >> >> > * The same is true for o.a.c.chain2.Chain and its base implemenation. For >> > example there is a race condition in >> o.a.c.chain2.impl.ChainBase.add(CMD). >> > The method first checks the forzen flag and then acts... Another thread >> may >> > already have altered frozen. I have created CHAIN-97 [2] for this. >> >> ok, gut! >> >> > * The use of Context<K, V> is inconsistent throughout the API. Commands >> > only need a C extends Map<K, V> for execution but the RemoveCommand >> > implements Command<K, V, Context<K, V>>. Why do we need the Context >> > interface anyway? It doesn't add much, the only method is just a generic >> > wrapper around Map.get(Object). >> >> Context must have disappeared from methods signatures some time ago, >> it is maybe a refuse - but at that time we agreed on creating a >> commodity interface which contains the <T extends V> T retrieve(K >> key); method; >> >> > * Base implementations should return the constants CONTINUE_PROCESSING or >> > PROCESSING_COMPLETE instead of returning true or false >> >> I'd even switch to enumeration to represent states... >> >> > * It may make sense to introduce an enum instead of working with booleans >> > as return values for commands. >> >> hahah nice one, I wrote the sentence above without reading the last >> bullet, sounds we have an agreement here... :) >> >> Thanks & Best, >> -Simo >> >> http://people.apache.org/~simonetripodi/ >> http://twitter.com/simonetripodi >> >> >> On Thu, Jun 20, 2013 at 1:01 PM, Benedikt Ritter <brit...@apache.org> >> wrote: >> > Hi guys, >> > >> > I had a look at the command interface and it's base classes yesterday, >> and >> > I think there is some work to do :) Here are the points I found: >> > >> > * JavaDoc of Command says that commands have to be designed in a >> > thread-safe manner, yet all the base implementations are not thread >> safe! I >> > have created CHAIN-96 [1] for this, because this definitely has to be >> fixed >> > before 2.0 >> > * The same is true for o.a.c.chain2.Chain and its base implemenation. For >> > example there is a race condition in >> o.a.c.chain2.impl.ChainBase.add(CMD). >> > The method first checks the forzen flag and then acts... Another thread >> may >> > already have altered frozen. I have created CHAIN-97 [2] for this. >> > * The use of Context<K, V> is inconsistent throughout the API. Commands >> > only need a C extends Map<K, V> for execution but the RemoveCommand >> > implements Command<K, V, Context<K, V>>. Why do we need the Context >> > interface anyway? It doesn't add much, the only method is just a generic >> > wrapper around Map.get(Object). >> > * Base implementations should return the constants CONTINUE_PROCESSING or >> > PROCESSING_COMPLETE instead of returning true or false >> > * It may make sense to introduce an enum instead of working with booleans >> > as return values for commands. >> > >> > Thoughts? >> > >> > Benedikt >> > >> > [1] https://issues.apache.org/jira/browse/CHAIN-96 >> > [2] https://issues.apache.org/jira/browse/CHAIN-97 >> > >> > -- >> > http://people.apache.org/~britter/ >> > http://www.systemoutprintln.de/ >> > http://twitter.com/BenediktRitter >> > http://github.com/britter >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
