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 > >
