kwin commented on code in PR #7:
URL:
https://github.com/apache/sling-org-apache-sling-rewriter/pull/7#discussion_r879263405
##########
src/main/java/org/apache/sling/rewriter/impl/ProcessorManagerImpl.java:
##########
@@ -240,6 +240,7 @@ protected void addProcessor(final String key, final String
configPath, final Pro
ConfigEntry[] newConfigs = new ConfigEntry[configs.length + 1];
System.arraycopy(configs, 0, newConfigs, 0, configs.length);
newConfigs[configs.length] = new ConfigEntry(configPath, config);
+ configs=newConfigs;
Review Comment:
```suggestion
configs = newConfigs;
```
##########
src/main/java/org/apache/sling/rewriter/impl/ProcessorManagerImpl.java:
##########
@@ -299,13 +300,11 @@ public synchronized void printConfiguration(final
PrintWriter pw) {
private synchronized void updateProcessor(final String path) {
final int pos = path.lastIndexOf('/');
final String key = path.substring(pos + 1);
- int keyIndex = 0;
// search the search path
for(final String sp : this.searchPath) {
Review Comment:
This loop doesn't seem to do anything any longer, right? Can't we remove it?
##########
src/main/java/org/apache/sling/rewriter/impl/ProcessorManagerImpl.java:
##########
@@ -327,61 +326,14 @@ private synchronized void updateProcessor(final String
path) {
}
if ( index != -1 ) {
// we are already in the array
- if ( index == 0 ) {
- // we are the first, so remove the old, and add the new
this.orderedProcessors.remove(configs[index].config);
Review Comment:
I would remove this optimization as well and would just call `addProcessor`
in all cases, that simplifies the code and should be neglectable from a
performance PoV
##########
src/main/java/org/apache/sling/rewriter/impl/ProcessorManagerImpl.java:
##########
@@ -327,61 +326,14 @@ private synchronized void updateProcessor(final String
path) {
}
if ( index != -1 ) {
// we are already in the array
- if ( index == 0 ) {
- // we are the first, so remove the old, and add the new
this.orderedProcessors.remove(configs[index].config);
configs[index] = new ConfigEntry(path, config);
if ( config.isActive() ) {
this.orderedProcessors.add(config);
Collections.sort(this.orderedProcessors, new
ProcessorConfiguratorComparator());
- }
- } else {
- // we are not the first, so we can simply exchange
- configs[index] = new ConfigEntry(path, config);
}
} else {
- // now we have to insert the new config at the correct
place
- int insertIndex = 0;
- boolean found = false;
- while ( !found && insertIndex < configs.length) {
- final ConfigEntry current = configs[insertIndex];
- int currentIndex = -1;
- for(int i=0; i<searchPath.length; i++) {
- if ( current.path.startsWith(searchPath[i]) ) {
- currentIndex = i;
- break;
- }
- }
- if ( currentIndex >= keyIndex ) {
- found = true;
- insertIndex = currentIndex;
- }
- }
-
- if ( !found ) {
- // just append
- this.addProcessor(key, path, config);
- } else {
- ConfigEntry[] newArray = new
ConfigEntry[configs.length + 1];
- int i = 0;
- for(final ConfigEntry current : configs) {
- if ( i == insertIndex ) {
- newArray[i] = new ConfigEntry(path, config);
- i++;
- }
- newArray[i] = current;
- i++;
- }
- this.processors.put(key, newArray);
- if ( insertIndex == 0 ) {
- // we are the first, so remove the old, and add
the new
- this.orderedProcessors.remove(configs[1].config);
- if ( config.isActive() ) {
- this.orderedProcessors.add(config);
- Collections.sort(this.orderedProcessors, new
ProcessorConfiguratorComparator());
- }
- }
- }
+ this.addProcessor(key, path, config);
Review Comment:
spaces instead of tab
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
