-
Notifications
You must be signed in to change notification settings - Fork 335
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
[BitSail][Feature] Support generate constant value in FakeSource #341
base: master
Are you sure you want to change the base?
Conversation
@@ -159,4 +173,107 @@ private Object fakeRawValue(TypeInfo<?> typeInfo) { | |||
} | |||
throw new RuntimeException("Unsupported type " + typeInfo); | |||
} | |||
|
|||
@SuppressWarnings("checkstyle:MagicNumber") | |||
private Object constantRawValue(TypeInfo<?> typeInfo, String constantValue) { |
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.
It seems that the code is redundant. Why not combine fakeRawValue and constantRawValue into one function
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.
thx for ur advice.
When I was writing, I also thought about whether to implement fakeRawValue and constantRawValue in one function, I thought fakeRawValue would be too long and hard to maintain for different logic mix together after combining, so I separated them, and the cost is that we got 2 functions that look similar
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.
After the merger, the code will be more concise, looking forward to your contribution @kyle-hawk
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.
okay, thx for the guidance, I'll merge them
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.
@lichang-bd I have merged them. please take a look. thx
Signed-off-by:
Pre-Checklist
Note: Please complete ALL items in the following checklist.
Purpose
Approaches
users can config constant value by defaultValue field, like this
Related Issues
New Behavior (screenshots if needed)
N/A