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]