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

optimize: remove repeated conditional tests (#3161) #3162

Merged
merged 3 commits into from
Oct 10, 2020

Conversation

everyhook1
Copy link
Contributor

@everyhook1 everyhook1 commented Sep 29, 2020

Ⅰ. Describe what this PR did

1.remove Repeated conditional tests
2.keySet().stream()->map.foreach

Ⅱ. Does this pull request fix one issue?

fixes #3161

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2020

Codecov Report

Merging #3162 into develop will decrease coverage by 0.09%.
The diff coverage is 83.33%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3162      +/-   ##
=============================================
- Coverage      50.36%   50.27%   -0.10%     
- Complexity      3114     3117       +3     
=============================================
  Files            594      594              
  Lines          19625    19603      -22     
  Branches        2438     2393      -45     
=============================================
- Hits            9885     9856      -29     
+ Misses          8744     8731      -13     
- Partials         996     1016      +20     
Impacted Files Coverage Δ Complexity Δ
...o/seata/integration/http/AbstractHttpExecutor.java 66.10% <83.33%> (+3.13%) 11.00 <2.00> (+1.00)
...obuf/convertor/BranchRegisterRequestConvertor.java 90.47% <0.00%> (-9.53%) 3.00% <0.00%> (ø%)
...otobuf/convertor/BranchReportRequestConvertor.java 90.90% <0.00%> (-9.10%) 3.00% <0.00%> (ø%)
...obuf/convertor/BranchRollbackRequestConvertor.java 92.00% <0.00%> (-8.00%) 3.00% <0.00%> (ø%)
...otobuf/convertor/GlobalBeginResponseConvertor.java 92.59% <0.00%> (-7.41%) 3.00% <0.00%> (ø%)
...otobuf/convertor/GlobalCommitRequestConvertor.java 93.75% <0.00%> (-6.25%) 3.00% <0.00%> (ø%)
...otobuf/convertor/GlobalStatusRequestConvertor.java 93.75% <0.00%> (-6.25%) 3.00% <0.00%> (ø%)
...obuf/convertor/GlobalRollbackRequestConvertor.java 93.75% <0.00%> (-6.25%) 3.00% <0.00%> (ø%)
...protobuf/convertor/RegisterTMRequestConvertor.java 94.73% <0.00%> (-5.27%) 3.00% <0.00%> (ø%)
...protobuf/convertor/RegisterRMRequestConvertor.java 90.47% <0.00%> (-4.77%) 3.00% <0.00%> (ø%)
... and 28 more

@funky-eyes funky-eyes added the first-time contributor first-time contributor label Sep 29, 2020
@@ -156,6 +155,12 @@
}

public static Map<String, String> convert(Map<String, Object> param) {
return param.keySet().stream().filter(key -> param.get(key) != null && param.get(key) != null).collect(Collectors.toMap(key -> key, key -> param.get(key).toString()));
Map<String, String> covertMap = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

recommend move this method to io.seata.common.util.CollectionUtils and make a appropriate method name?
建议把这个方法移动到 io.seata.common.util.CollectionUtils,然后起个更合理的名字。

Map<String, String> covertMap = new HashMap<>();
param.forEach((key, value) -> {
if (value != null) {
covertMap.put(key, value.toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

recommend StringUtils.objectToString instead of value.toString()
建议StringUtils.objectToString替代value.toString

Copy link
Member

@xingfudeshi xingfudeshi left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

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

LGTM

@slievrly slievrly merged commit ed98876 into apache:develop Oct 10, 2020
@wangliang181230 wangliang181230 added this to the 1.4.0 milestone Oct 19, 2020
l81893521 pushed a commit to l81893521/seata that referenced this pull request Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-time contributor first-time contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AbstractHttpExecutor Map<String, String> convert(Map<String, Object> param) Repeated conditional tests
7 participants