Skip to content

refactor(Joiner): rename internal toString helper to toCharSequence#8519

Open
Derylfabiensyah wants to merge 2 commits into
google:masterfrom
Derylfabiensyah:refactor/joiner-rename-tostring-helper
Open

refactor(Joiner): rename internal toString helper to toCharSequence#8519
Derylfabiensyah wants to merge 2 commits into
google:masterfrom
Derylfabiensyah:refactor/joiner-rename-tostring-helper

Conversation

@Derylfabiensyah

Copy link
Copy Markdown

Renamed the package-private helper method toString(Object) to toCharSequence(Object) in Joiner.java to resolve cpovirk's TODO.

This clarifies the return type (CharSequence) and avoids shadowing the standard Object.toString() signature for internal calls. Both the standard and Android version source trees have been updated.

@ricardoofnl ricardoofnl left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice mechanical rename. I checked all call sites in both trees and nothing was missed. There is one small leftover, which I flagged inline in both files.


// TODO(cpovirk): Rename to "toCharSequence."
CharSequence toString(@Nullable Object part) {
CharSequence toCharSequence(@Nullable Object part) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The comment block further down in this method (around line 524) still references the old name:

Its implementation avoids calling this toString(Object) method in the first place.

Since this PR renames the method, it would be good to update that mention to toCharSequence(Object) as well.


// TODO(cpovirk): Rename to "toCharSequence."
CharSequence toString(@Nullable Object part) {
CharSequence toCharSequence(@Nullable Object part) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same as in the JRE flavor: the comment block further down in this method (around line 488) still says toString(Object) and should be updated to toCharSequence(Object) as part of this rename.

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