Skip to content

Conversation

choi-hyeseong
Copy link

The File.getName() method always slice the path to get the name. so, it's not cached
It doesn't have a huge impact, but it's not efficient, so i fixed it :)

Also, the original code was truncating the extension from the filename, adding the namespace, and then putting the extension back together again, but i modified the code to be cleaner by using StringBuilder to do the insertion.

This change passed the test (WebServerPortFileWriterTests.java)

  • Remove duplicated File.getName() method
  • using StringBuilder to insert namespace

@pivotal-cla
Copy link

@choi-hyeseong Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@choi-hyeseong Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 22, 2024
@philwebb philwebb self-assigned this Sep 23, 2024
@philwebb philwebb added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 23, 2024
@philwebb philwebb added this to the 3.4.0-RC1 milestone Sep 23, 2024
philwebb pushed a commit that referenced this pull request Sep 23, 2024
Update `WebServerPortFileWriter` so that `file.getName()` is only called
once.

See gh-42411
philwebb added a commit that referenced this pull request Sep 23, 2024
@philwebb philwebb closed this in 65a72c0 Sep 23, 2024
@philwebb
Copy link
Member

Thanks very much @choi-hyeseong, I wasn't aware that calls to getName() aren't cached. I'm not totally sure about using a StringBuilder here, to me it makes things a little harder to read. I've changed things back to using Strings, but tried to improve the variable names a little in 60f6158.

@choi-hyeseong
Copy link
Author

Thank you for considering my PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants