Hi Antonio

You are not grumpy. See my comments inline

Antonio Gallardo wrote:
[EMAIL PROTECTED] wrote:

Author: chestnut
Date: Mon Dec 12 21:50:01 2005
New Revision: 356483

URL: http://svn.apache.org/viewcvs?rev=356483&view=rev
Log:
fixes broken compilation: IndexConfiguration (Bug 37873)
Thanks to Felix Röthenbacher for the patch


Modified:
lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java

Modified: lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java URL: http://svn.apache.org/viewcvs/lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java?rev=356483&r1=356482&r2=356483&view=diff ============================================================================== --- lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java (original) +++ lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java Mon Dec 12 21:50:01 2005
@@ -20,10 +20,10 @@
import java.io.File;
import java.io.IOException;

-import org.apache.avalon.excalibur.io.FileUtil;
import org.apache.avalon.framework.configuration.Configuration;
import org.apache.avalon.framework.configuration.ConfigurationException;
import org.apache.avalon.framework.configuration.DefaultConfigurationBuilder;
+import org.apache.commons.io.FilenameUtils;
import org.apache.log4j.Logger;
import org.xml.sax.SAXException;

@@ -155,9 +155,11 @@
     * @return A string.
     */
    public String resolvePath(String path) {
-        if (path.indexOf(File.separator) == 0) {
-            return path;
+        File pathName = new File(path);
+        if (pathName.isAbsolute()) {
+            return pathName.getAbsolutePath();
+        } else {
+ return FilenameUtils.concat(this.configurationFilePath, path);
        }
-        return FileUtil.catPath(this.configurationFilePath, path);
    }
}

Sorry for being grumpy. ;-)

There is no need to create a "new File" inside this method.
It only have a performance penality.

The method is rarely called, I can't see any performance penalty here.
Basically, creating a File object is nothing bad, it's just an
accessor object, no system resources are involved in this stage.

Can we don just replace:

return FileUtil.catPath(this.configurationFilePath, path);

with:

return FilenameUtils.concat(this.configurationFilePath, path);

and keep the method as it was?

Quote from http://jakarta.apache.org/commons/io/apidocs/org/apache/commons/io/FilenameUtils.html

  "NOTE: You may be able to avoid using this class entirely simply by
   using JDK File objects and the two argument constructor
   File(File,String)."

There is no need to use this helper class, it's just an additional
dependency for a plain debug message. So I would suggest to avoid its
usage at all.

There was a check for absolute paths in the original method:

>>     public String resolvePath(String path) {
>> -        if (path.indexOf(File.separator) == 0) {
>> -            return path;

I replaced it with a simpler, more readable statement.
You are right, this check is unnecessary. But we should check wether
this.configurationFilePath is a directory or a file otherwise
we end up with something that will not work.


Best

Felix


WDYT?

Best Regards,

Antonio Gallardo.


--
Felix Röthenbacher                  [EMAIL PROTECTED]
Wyona Inc.  -   Open Source Content Management   -   Apache Lenya
http://www.wyona.com                      http://lenya.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to