geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeremy Boynes <jboy...@apache.org>
Subject Re: Signatrue change of GBeanRegistry.listGBeans [Re: svn commit: r154941]
Date Thu, 24 Feb 2005 02:35:32 GMT
Dain Sundstrom wrote:
> On Feb 23, 2005, at 6:00 PM, Jeremy Boynes wrote:
> 
>> Dain Sundstrom wrote:
>>
>>> No. I'm saying we have a status quo that people understand and code 
>>> to.  You are arguing to change the this so, I believe the burden is 
>>> on you to prove that we are better off removing the features.
>>
>>
>> You yourself said that no-one is using it, so removing the complexity 
>> and simplifying the implementation is normally considered a good thing.
> 
> 
> My comment was not just about domain queries, it also addressed the 
> split of names and patterns, which you comment does not address.
> 

Well you cut everything else from the email ...

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

> 
> Is there some other reason you want to remove these feature?
> 

Nope - the previous implementation was needlessly complex given it was 
not actually used.

KISS
--
Jeremy

Code from previous version:

         String patternDomain = pattern.getDomain();
         if (patternDomain.length() == 0) {
             patternDomain = defaultDomainName;
         }

         // work with a copy of the registry key set
         List nameToProperties;
         if (!pattern.isDomainPattern()) {
             synchronized (this) {
                 // create an array list big enough to match all 
names... extra space is better than resizing
                 nameToProperties = new ArrayList(registry.size());

                 // find we are only matching one specific domain, so
                 // just grab it directly from the index
                 Map map = (Map) domainIndex.get(patternDomain);
                 if (map != null) {
                     nameToProperties.addAll(map.entrySet());
                 }
             }
         } else if (patternDomain.equals("*")) {
             // this is very commmon, so support it directly
             synchronized (this) {
                 // create an array list big enough to match all 
names... extra space is better than resizing
                 nameToProperties = new ArrayList(registry.size());

                 // find we are matching all domain, so just grab all of 
them directly
                 for (Iterator iterator = 
domainIndex.values().iterator(); iterator.hasNext();) {
                     Map map = (Map) iterator.next();

                     // we can just copy the entry set directly into the 
list we don't
                     // have to worry about duplicates as the maps are 
mutually exclusive
                     nameToProperties.addAll(map.entrySet());
                 }
             }
         } else {
             String perl5Pattern = domainPatternToPerl5(patternDomain);
             Pattern domainPattern = Pattern.compile(perl5Pattern);

             synchronized (this) {
                 // create an array list big enough to match all 
names... extra space is better than resizing
                 nameToProperties = new ArrayList(registry.size());

                 // find all of the matching domains
                 for (Iterator iterator = 
domainIndex.entrySet().iterator(); iterator.hasNext();) {
                     Map.Entry entry = (Map.Entry) iterator.next();
                     String domain = (String) entry.getKey();
                     if (domainPattern.matcher(domain).matches()) {
                         // we can just copy the entry set directly into 
the list we don't
                         // have to worry about duplicates as the maps 
are mutually exclusive
                         Map map = (Map) entry.getValue();
                         nameToProperties.addAll(map.entrySet());
                     }
                 }
             }
         }

     private static String domainPatternToPerl5(String pattern) {
         char[] patternCharacters = pattern.toCharArray();
         StringBuffer buffer = new StringBuffer(2 * 
patternCharacters.length);
         for (int position = 0; position < patternCharacters.length; 
position++) {
             char character = patternCharacters[position];
             switch (character) {
                 case '*':
                     // replace '*' with '.*'
                     buffer.append(".*");
                     break;
                 case '?':
                     // replace '?' with '.'
                     buffer.append('.');
                     break;
                 default:
                     // escape any perl5 characters with '\'
                     if (isPerl5MetaCharacter(character)) {
                         buffer.append('\\');
                     }
                     buffer.append(character);
                     break;
             }
         }

         return buffer.toString();
     }

     private static boolean isPerl5MetaCharacter(char character) {
         return ("'*?+[]()|^$.{}\\".indexOf(character) >= 0);
     }

Mime
View raw message