Skip to content
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

refactor(core): improve backend code by flow idea suggests #2000

Merged
merged 2 commits into from
Nov 11, 2022

Conversation

jadepeng
Copy link
Contributor

@jadepeng jadepeng commented Nov 2, 2022

No description provided.

@codecov
Copy link

codecov bot commented Nov 2, 2022

Codecov Report

Merging #2000 (516579d) into master (b1b1209) will increase coverage by 3.97%.
The diff coverage is 76.00%.

@@             Coverage Diff              @@
##             master    #2000      +/-   ##
============================================
+ Coverage     62.41%   66.38%   +3.97%     
  Complexity      976      976              
============================================
  Files           482      482              
  Lines         41450    41449       -1     
  Branches       5896     5890       -6     
============================================
+ Hits          25870    27516    +1646     
+ Misses        13034    11232    -1802     
- Partials       2546     2701     +155     
Impacted Files Coverage Δ
.../com/baidu/hugegraph/backend/BackendException.java 53.84% <ø> (ø)
...ugegraph/backend/cache/CachedGraphTransaction.java 85.71% <ø> (+5.29%) ⬆️
...va/com/baidu/hugegraph/backend/id/IdGenerator.java 77.20% <ø> (+2.94%) ⬆️
...idu/hugegraph/backend/id/SnowflakeIdGenerator.java 69.49% <ø> (+0.63%) ⬆️
...a/com/baidu/hugegraph/backend/query/Condition.java 79.09% <ø> (+0.69%) ⬆️
...hugegraph/backend/query/ConditionQueryFlatten.java 75.73% <ø> (-0.11%) ⬇️
...egraph/backend/serializer/BinaryEntryIterator.java 87.03% <ø> (+25.92%) ⬆️
...ugegraph/backend/serializer/SerializerFactory.java 85.18% <ø> (ø)
...ugegraph/backend/serializer/TableBackendEntry.java 88.88% <ø> (ø)
...aidu/hugegraph/backend/serializer/BytesBuffer.java 89.16% <50.00%> (ø)
... and 69 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

StringBuilder sb = new StringBuilder(1 + idString.length());
sb.append(id.type().prefix()).append(idString);
return sb.toString();
return id.type().prefix() + idString;
Copy link
Member

Choose a reason for hiding this comment

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

should warn if it influence the performance. some suggestions are not perf friendly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里就两个string,用StringBuilder代价更大

Copy link
Member

Choose a reason for hiding this comment

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

也是, 而且高版本 JDK 编译器似乎已经会自动判断开销转换了. (当然我不确定是从 JDK 几开始的)

imbajin
imbajin previously approved these changes Nov 7, 2022
@javeme
Copy link
Contributor

javeme commented Nov 8, 2022

@jadepeng you can use git rebase and git push -f to update the code

@imbajin
Copy link
Member

imbajin commented Nov 8, 2022

@jadepeng you can use git rebase and git push -f to update the code

image

The github UI will alert update with the master automatically (I have clicked it)

It is safer and doesn't need to review the code again (avoid unnecessary manual rebase operations)

TODO: I'll enable the required in CI after all package renamed

zyxxoo
zyxxoo previously approved these changes Nov 9, 2022
@jadepeng jadepeng dismissed stale reviews from zyxxoo and imbajin via b681523 November 11, 2022 08:28
@jadepeng jadepeng force-pushed the impove-backend-code branch from ae5e0b2 to b681523 Compare November 11, 2022 08:28
@javeme javeme merged commit 020b7dc into apache:master Nov 11, 2022
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.

4 participants