Skip to content

Conversation

@jrenaat
Copy link
Member

@jrenaat jrenaat commented Dec 5, 2025


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.


https://hibernate.atlassian.net/browse/HHH-19969

Signed-off-by: Jan Schatteman <jschatte@redhat.com>
@jrenaat jrenaat requested a review from gavinking December 9, 2025 13:30
Comment on lines +63 to 74
final char[] copy = builder.toString().toCharArray();
int builderIndex = 1;
for ( int i = 1; i <= copy.length - 1; i++ ) {
if ( isUnderscoreRequired(
copy[i - 1],
copy[i],
i == copy.length - 1 ? '*' : copy[i + 1] ) // add some fictitious final character
) {
builder.insert( builderIndex++, '_' );
}
builderIndex++;
}
Copy link
Member

@beikov beikov Dec 12, 2025

Choose a reason for hiding this comment

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

It seems to me that this would be simpler and avoid creating the char array copy.

Suggested change
final char[] copy = builder.toString().toCharArray();
int builderIndex = 1;
for ( int i = 1; i <= copy.length - 1; i++ ) {
if ( isUnderscoreRequired(
copy[i - 1],
copy[i],
i == copy.length - 1 ? '*' : copy[i + 1] ) // add some fictitious final character
) {
builder.insert( builderIndex++, '_' );
}
builderIndex++;
}
for ( int i = 1; i < builder.length() - 1; i++ ) {
if ( isUnderscoreRequired( builder.charAt( i - 1 ), builder.charAt( i ), builder.charAt( i + 1 ) ) ) {
builder.insert( i++, '_' );
}
}
if ( builder.length() > 1 && isUnderscoreRequired( builder.charAt(builder.length() - 2), builder.charAt(builder.length() - 1), '0' ) ) {
builder.insert( builder.length() - 1, '_' );
}

Copy link
Member Author

@jrenaat jrenaat Dec 12, 2025

Choose a reason for hiding this comment

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

I used the char[] copy to avoid the index increment inside the for loop, which is what caused the jump over the last character (and it makes it somewhat hard to follow, and it's imo ugly (arguable))
Also, your change still uses the copy ?

Copy link
Member

Choose a reason for hiding this comment

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

Also, your change still uses the copy ?

Yeah, forgot to adapt a copy pasted part.

I used the char[] copy to avoid the index increment inside the for loop, which is what caused the jump over the last character (and it makes it somewhat hard to follow, and it's imo ugly (arguable))

Dunno if it's "ugly", but let's focus on the bug at hand. It seems to me that the problem is/was rather that the last character of the string was not properly considered, and just checking the last character as "current" character is the way to go to understand if an underscore is needed. Am I wrong?

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