Skip to content

Conversation

@verhasi
Copy link

@verhasi verhasi commented Nov 15, 2024

No functional modification applied just simplified the implementation to make it more readable.

*/
private static String removeLeadingSlash(String name)
{
return name.replaceFirst("^/*", "");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the benefit to move a single line to method which is used only once?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moveover, this requires compiling a regexp every single time.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't startsWith and substring cheap enough?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can go ahead with startsWith and substring. I propose the following:

    /**
     * remove leading slash(es) so path will work with classes in a JAR file
     */
    private static String removeLeadingSlash(String name)
    {
        int i = 0;
        while (name.startsWith("/",i)) i++;
        return i>0 ? name.substring(i) : name;
    }

It performs the same in case of 0 or 1 slash, but I am sure it is faster in case the number of slashes is greater than 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants