geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dain Sundstrom <dsundst...@gluecode.com>
Subject Re: Signatrue change of GBeanRegistry.listGBeans [Re: svn commit: r154941]
Date Thu, 24 Feb 2005 03:14:06 GMT
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


Mime
View raw message