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

[ISSUE #9282]remove redundant judgment #9284

Merged
merged 1 commit into from
Oct 10, 2022

Conversation

CLFutureX
Copy link
Contributor

@CLAassistant
Copy link

CLAassistant commented Oct 9, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@KomachiSion KomachiSion left a comment

Choose a reason for hiding this comment

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

How about cacheMap.get() is null or configChangeBatchListenResponse is null?

@CLFutureX
Copy link
Contributor Author

CLFutureX commented Oct 10, 2022

How about cacheMap.get() is null or configChangeBatchListenResponse is null?

1 As a global variable, cacheMap is initialized when it is created. So cacheMap.get() will not be empty
private final AtomicReference<Map<String, CacheData>> cacheMap = new AtomicReference<>(new HashMap<>());

@CLFutureX
Copy link
Contributor Author

2 requestProxy()-->rpcClientInner.request
image
If it is null, a null exception will be thrown in advance, so it will not return null

You can also see that there is no null judgment in other places where this method is called.
for example
image

@codecov-commenter
Copy link

Codecov Report

Merging #9284 (7b0d55f) into develop (5396858) will not change coverage.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             develop    #9284   +/-   ##
==========================================
  Coverage      44.08%   44.08%           
+ Complexity      4858     4857    -1     
==========================================
  Files            913      913           
  Lines          33047    33047           
  Branches        3825     3826    +1     
==========================================
  Hits           14569    14569           
+ Misses         17059    17055    -4     
- Partials        1419     1423    +4     
Impacted Files Coverage Δ
...alibaba/nacos/client/config/impl/ClientWorker.java 55.16% <0.00%> (-0.20%) ⬇️
.../core/v2/event/publisher/NamingEventPublisher.java 68.75% <0.00%> (-6.25%) ⬇️
...alibaba/nacos/core/remote/control/TpsRecorder.java 74.32% <0.00%> (-1.36%) ⬇️
...baba/nacos/naming/healthcheck/HealthCheckTask.java 45.58% <0.00%> (ø)
...m/alibaba/nacos/core/remote/ConnectionManager.java 44.37% <0.00%> (+1.47%) ⬆️
...os/client/auth/ram/identify/CredentialService.java 82.69% <0.00%> (+3.84%) ⬆️

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 5396858...7b0d55f. Read the comment docs.

@KomachiSion KomachiSion merged commit c3b38a0 into alibaba:develop Oct 10, 2022
@KomachiSion KomachiSion added this to the 2.1.2 milestone Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants