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