verhasi commented on code in PR #56:
URL: https://github.com/apache/velocity-engine/pull/56#discussion_r1850391300
##########
velocity-engine-core/src/main/java/org/apache/velocity/util/ClassUtils.java:
##########
@@ -107,53 +107,43 @@ public static Object getNewInstance(String clazz)
/**
* Finds a resource with the given name. Checks the Thread Context
- * classloader, then uses the System classloader. Should replace all
- * calls to <code>Class.getResourceAsString</code> when the resource
+ * classloader, then uses the System classloader (for compatibility with
texen / ant tasks)
+ * Should replace all calls to <code>Class.getResourceAsString</code> when
the resource
* might come from a different classloader. (e.g. a webapp).
- * @param claz Class to use when getting the System classloader (used if
no Thread
+ * @param clazz Class to use when getting the System classloader (used if
no Thread
* Context classloader available or fails to get resource).
* @param name name of the resource
* @return InputStream for the resource.
*/
- public static InputStream getResourceAsStream(Class<?> claz, String name)
+ public static InputStream getResourceAsStream(Class<?> clazz, String name)
{
InputStream result = null;
- /*
- * remove leading slash so path will work with classes in a JAR file
- */
- while (name.startsWith("/"))
- {
- name = name.substring(1);
- }
+ name = removeLeadingSlash(name);
- ClassLoader classLoader = Thread.currentThread()
- .getContextClassLoader();
+ ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
- if (classLoader == null)
+ if (classLoader != null)
{
- classLoader = claz.getClassLoader();
result = classLoader.getResourceAsStream( name );
}
- else
- {
- result= classLoader.getResourceAsStream( name );
-
- /*
- * for compatibility with texen / ant tasks, fall back to
- * old method when resource is not found.
- */
- if (result == null)
- {
- classLoader = claz.getClassLoader();
- if (classLoader != null)
- result = classLoader.getResourceAsStream( name );
- }
+ if (result == null)
+ {
+ classLoader = clazz.getClassLoader();
+ if (classLoader != null)
+ result = classLoader.getResourceAsStream( name );
}
return result;
+ }
+ /**
+ * remove leading slash(es) so path will work with classes in a JAR file
+ */
+ private static String removeLeadingSlash(String name)
+ {
+ return name.replaceFirst("^/*", "");
Review Comment:
The trigger for moving to a separate method was the smelling inline comment
(that is not a Javadoc). At first, it was three lines of code and after moving
I refactored it to an oneliner. I still see it as better readable as the method
has a meaningful name.
Regarding the performance. The starting point was a cycle that created a new
String object for each starting slash. Depending on the number of starting
slashes the replaceFirst is an enhancement. On the other hand, I agree with you
this could be split into a static compiled pattern and a matching part.
--
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: dev-unsubscr...@velocity.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
For additional commands, e-mail: dev-h...@velocity.apache.org
