-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider both annotation - current field and parent #118
base: main
Are you sure you want to change the base?
Consider both annotation - current field and parent #118
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment
#117 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor change in naming requested
@@ -80,4 +80,17 @@ | |||
*/ | |||
@ExperimentalApi(issue = "https://github.com/ydb-platform/yoj-project/issues/24") | |||
CustomValueType customValueType() default @CustomValueType(columnClass = Comparable.class, converter = NoConverter.class); | |||
|
|||
enum NamingStrategy { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There already is a class named NamingStrategy. Let's rename this enum into ColumnNaming or something like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
return (field.getParent() == null) | ||
? field.getField().getName() | ||
: field.getParent().getName() + NAME_DELIMITER + field.getField().getName(); | ||
if (annotation.namingStrategy() == Column.NamingStrategy.ABSOLUTE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using expression switch
here will produce clearer code, I think:
var finalColumnName = switch (annotation.columnNaming()) {
case ABSOLUTE –> ...
case RELATIVE -> ...
case LEGACY -> ...
};
return finalColumnName;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Plus extracted method for improving readability
Fix of problem described here: #117