Skip to content

Simplify some methods for easy understanding#2920

Merged
gabry-lab merged 21 commits intoapache:devfrom
CalvinKirs:messy
Jun 21, 2020
Merged

Simplify some methods for easy understanding#2920
gabry-lab merged 21 commits intoapache:devfrom
CalvinKirs:messy

Conversation

@CalvinKirs
Copy link
Copy Markdown
Member

Tips

What is the purpose of the pull request

1:Deprecated method replace
2:Simplify some methods for easy understanding

Brief change log

Small changes

Verify this pull request

(Please pick either of the following options)

This pull request is already covered by existing tests, such as (please describe tests).

@CalvinKirs CalvinKirs changed the title messy Simplify some methods for easy understanding Jun 7, 2020
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 7, 2020

Codecov Report

Merging #2920 into dev will decrease coverage by 0.20%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #2920      +/-   ##
============================================
- Coverage     39.31%   39.11%   -0.21%     
+ Complexity     2700     2688      -12     
============================================
  Files           435      435              
  Lines         20123    20114       -9     
  Branches       2453     2452       -1     
============================================
- Hits           7912     7868      -44     
- Misses        11458    11494      +36     
+ Partials        753      752       -1     
Impacted Files Coverage Δ Complexity Δ
...dolphinscheduler/common/utils/CollectionUtils.java 92.53% <100.00%> (ø) 30.00 <0.00> (ø)
.../dolphinscheduler/common/utils/DependentUtils.java 95.00% <100.00%> (-0.17%) 28.00 <0.00> (ø)
.../apache/dolphinscheduler/common/utils/IpUtils.java 90.90% <100.00%> (-2.43%) 2.00 <0.00> (ø)
...che/dolphinscheduler/common/utils/SchemaUtils.java 82.60% <100.00%> (+5.05%) 13.00 <0.00> (-1.00) ⬆️
.../server/worker/processor/TaskExecuteProcessor.java 10.16% <0.00%> (-27.12%) 1.00% <0.00%> (-2.00%)
...nscheduler/server/entity/TaskExecutionContext.java 89.10% <0.00%> (-9.91%) 61.00% <0.00%> (-5.00%)
...he/dolphinscheduler/common/thread/ThreadUtils.java 68.25% <0.00%> (-9.53%) 13.00% <0.00%> (-1.00%)
...org/apache/dolphinscheduler/remote/utils/Host.java 28.57% <0.00%> (-5.72%) 6.00% <0.00%> (-1.00%)
...er/master/processor/queue/TaskResponseService.java 65.30% <0.00%> (-4.09%) 5.00% <0.00%> (-1.00%)
...e/dolphinscheduler/remote/NettyRemotingClient.java 53.95% <0.00%> (-1.44%) 10.00% <0.00%> (-1.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8031c16...ec41f02. Read the comment docs.

@gabry-lab
Copy link
Copy Markdown
Member

@CalvinKirs can you resolve the failed checks

@CalvinKirs
Copy link
Copy Markdown
Member Author

@CalvinKirs can you resolve the failed checks

Interestingly, commons-beanutils actually detects LICENSE failure

Map<String, String> map = new HashMap<>(strings.length);
for (int i = 0; i < strings.length; i++) {
String[] strArray = strings[i].split("=");
for (String string : strings) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I do not think foreach is better than fori ,especial for []

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I do not think foreach is better than fori ,especial for []

When he doesn’t use the index, it’s not so elegant to use this form, what do you think?

Copy link
Copy Markdown
Member

@gabry-lab gabry-lab Jun 15, 2020

Choose a reason for hiding this comment

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

I don't think so .Using index to traverse array [] is a standard way ,right ? Why is it not elegant?
foreach is better than fori when traversing List, Set, Array ,etc.

Copy link
Copy Markdown
Member

@gabry-lab gabry-lab left a comment

Choose a reason for hiding this comment

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

Please give more details about the reason changing the code ?

@CalvinKirs
Copy link
Copy Markdown
Member Author

Please give more details about the reason changing the code ?

I have added instructions, please look again, tks

@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 2 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (1.8.0_252) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

@CalvinKirs
Copy link
Copy Markdown
Member Author

@gabrywu Thank you very much for your suggestion, I have completed the change, please review it, tks

Copy link
Copy Markdown
Member

@gabry-lab gabry-lab left a comment

Choose a reason for hiding this comment

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

+1

@gabry-lab gabry-lab merged commit 518d03b into apache:dev Jun 21, 2020
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.

3 participants