On Mon, 5 May 2025 17:32:19 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> Refactor AbstractStringBuilder to maintain consistency among count, coder, >> and value buffers while the buffer capacity is being expanded and/or >> inflated from Latin1 to UTF16 representations. >> The refactoring pattern is to read and write AbstractStringBuilder fields >> once using locals for all intermediate values. >> Support methods are static, designed to pass all values as arguments and >> return a value. >> >> The value byte array is reallocated under 3 conditions: >> - Increasing the capacity with the same encoder >> - Increasing the capacity and inflation to change the coder from LATIN1 to >> UTF16 >> - Inflation with the same capacity >> >> Added StressSBTest to exercise public instance methods of StringBuilder. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Refactor to consistently use `isLatin1(coder)` within AbstractStringBuilder. > The refactoring pattern is to read and write AbstractStringBuilder fields > once using locals for all intermediate values. I see that in the proposed changes, we are now using the same names for these local variables and method parameters as the field names. Would using different names for these local variables be better? To avoid shadowing the field names (`value`, `coder` and `count`)? I think that could help prevent future (accidental) refactoring changes that end up unintentionally accessing fields instead of local variables. ------------- PR Comment: https://git.openjdk.org/jdk/pull/24967#issuecomment-2854499902
