Skip to content

Conversation

kyle-hawk
Copy link
Contributor

Signed-off-by:

Pre-Checklist

Note: Please complete ALL items in the following checklist.

  • I have read through the CONTRIBUTING.md documentation.
  • My code has the necessary comments and documentation (if needed).
  • I have added relevant tests.

Purpose

Approaches

users can config constant value by defaultValue field, like this

      "columns": [
        {
          "name": "double_value",
          "type": "double",
          "defaultValue": "13.5"
        }
      ]

Related Issues

New Behavior (screenshots if needed)

N/A

}

@SuppressWarnings("checkstyle:MagicNumber")
private Object constantRawValue(TypeInfo<?> typeInfo, String constantValue) {
Copy link
Member

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

Copy link
Contributor Author

@kyle-hawk kyle-hawk Feb 2, 2023

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

Copy link
Member

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

Copy link
Contributor Author

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

Copy link
Contributor Author

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

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.

[BitSail][Feature] Support generate constant value in FakeSource.

2 participants