On Aug 29, 2013, at 4:28 AM, bugzi...@apache.org wrote: > https://issues.apache.org/bugzilla/show_bug.cgi?id=55317 > > --- Comment #16 from Mark Thomas <ma...@apache.org> --- > (In reply to Nick Williams from comment #15) >> Created attachment 30749 [details] >> Proposed implementation of this feature > > 1. Why loop over list rather than using contains() in addTransformer() ? > > 2. Should addTransformer() be looking for multiple instances of the same > Transformer or multiple instances of the same class of Transformer? > > 3. Why not use List.remove(Object) in removeTransformer() ? > > 4. I'm concerned that synchronizing on the list of transformers while classes > are transformed will become a bottleneck when lots of classes are being loaded > and the transformer is relatively slow. A separate ReadWriteLock for the > transformer list is probably the way to go but really some testing is required > to determine if there is an issue here or not. > > 5. I'm not a fan of the org.apache.tomcat.unittest package unless the classes > concerned are going to be used by multiple tests across multiple packages.
FYI, I replied to your comments and submitted a revised patch. It doesn't appear that Bugzilla sent out an email this time. Not sure what happened, but I wanted to make sure you were notified somehow so that you could take a look. Nick
smime.p7s
Description: S/MIME cryptographic signature
