On Feb 23, 2005, at 6:35 PM, Jeremy Boynes wrote:
Well you cut everything else from the email ...
Sorry missed that:
Same with splitting the name - that's not removing functionality, its refactoring the API to make it easier to use by eliminating the overlap between values and patterns (given, even in JMX, patterns proved inadequate and lead to the introduction of QueryExp).
Well I guess it depends on how you are proposing to implement it. If it involves a sub class, then I guess it doesn't matter. If it does not involve a subclass, then it means I can not handle an object name and a pattern using the same code. Anyway, I still believe the burden is on you to show that this is simpler then having a GBeanName supporting patterns out of the box (just like object name).
Saying that "removing complexity is good" is like saying "puppies are cute". Yes we all agree, but that does not prove that this feature results in complexity, and that removing it will result in less complexity. Complexity is a hydra, you chop off one head and it reappears somewhere else.
I personally do not find the domain query complex and we already have a fully working implementation in the non-jmx registry. I do not think that removing it will make this class noticeably simpler, but maybe you are thinking of some other code.
I'm thinking of the code that checked if the domain was pattern (involving a String scan), did a character substitution to convert it to a Perl regex (multiple String scans), and then applied that Pattern to all values (some state machine in the VM).
See the code at the end of this mail or at:
http://svn.apache.org/viewcvs.cgi/geronimo/trunk/modules/kernel/src/ java/org/apache/geronimo/kernel/registry/AbstractGBeanRegistry.java? rev=151106&view=markup
Or, which delegated to MBeanServer.queryMBeans() and believe me I know the gyrations that goes through (never mind you're querying all registered MBeans not just GBeans) including a variety of security checks.
Compared to the revised implementation: if (domain != null) { if (!this.domain.equals(domain)) { return false; } }
I just looked at the code again, and it is simple to me and well documented. The code is optimized for the case where we are not using a pattern, and for the case where we are using a pattern, I believe the implementation is reasonably fast. If not, that can be corrected as an internal detail of the code. Also, this feature is isolated to *one* class in Geronimo, so I really don't see this nasty complexity you want to cut out.
Is there some other reason you want to remove these feature?
Nope - the previous implementation was needlessly complex given it was not actually used.
In that case, please leave it alone.
-dain
