Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upRefactoring: use NamingHelper inside of Constraint #2992
Conversation
…t.hashedName()
…comparator class
… you had got a NPE on sorting
…move concatenation inside it's constructor, use append(char) as more faster than append(String). Do the same optimization for the similar NamingHelper.generateHashedConstraintName()
…ConstraintName()
… already comparable by the getCanonicalName
…ing of columns
| // they were bound. | ||
| Arrays.stream( columnNames ) | ||
| .sorted() | ||
| .forEachOrdered( columnName -> sb.append( "column`" ).append( columnName ).append( '`' ) ); |
This comment has been minimized.
This comment has been minimized.
NathanQingyangXu
Sep 2, 2020
Contributor
The above code snippet might be slower than traditional approach. It would be great to conduct some JMH benchmark to justify the stream usage.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
stokito commentedAug 17, 2019
•
edited
Here is a small refactoring that can improve speed a little bit and removes duplicated code.
There is a lot of code duplication between
ConstraintandNamingHelperrelated to generation of constraint name.I did step by step refactoring and it turned out that
Constraintcan just reuse theNamingHelper.I kept an API and all tests passed.
There is a tricky part in #23833fd because the
Columnclass doesn't have anIdentifier(IMHO it worth to create one) so we'll create them manually.Also I worried that the Table.name and Column.name fields can be in uppercase while the NamingHelper always lowercase them before hashing. This may change the generated hash.
But I checked and it turned out that the Table.name and Column.name fields are always in lowercase.
So this PR should be safe to merge.
BTW: the
Identifier.compareTo()method compares bygetCanonicalNamewhich looks likeIt looks like for columns the
textwill be always in lowercase so maybe this can be simplified