[Dubbo-3625] Add constant 'MIN_PATH_ARRAY_LENGTH' #3680
Conversation
| if (done != null) { | ||
| done.signal(); | ||
| } | ||
| done.signalAll(); |
There was a problem hiding this comment.
Hi, this change is irrelevant. And is addressed in #3681 . Please rebase with the master and send the pull request again.
There was a problem hiding this comment.
OK，I will send the pull request again.
Codecov Report
@@ Coverage Diff @@
## master #3680 +/- ##
============================================
- Coverage 63.44% 63.31% -0.14%
Complexity 71 71
============================================
Files 703 703
Lines 31000 31050 +50
Branches 5024 5039 +15
============================================
- Hits 19669 19659 -10
- Misses 9066 9122 +56
- Partials 2265 2269 +4
Continue to review full report at Codecov.
|
| @@ -89,7 +89,8 @@ public void dataChanged(String path, Object value, EventType eventType) { | |||
| // TODO We limit the notification of config changes to a specific path level, for example | |||
| // /dubbo/config/service/configurators, other config changes not in this level will not get notified, | |||
| // say /dubbo/config/dubbo.properties | |||
| if (path.split("/").length >= 5) { | |||
| final int MIN_PATH_ARRAY_LENGTH = 5; | |||
There was a problem hiding this comment.
It is better to make it static.
There was a problem hiding this comment.
It is better to be local, because it is only used in one method.
There was a problem hiding this comment.
No, it is not. As a local constant, it will be allocated every time the method is invoked. I don't think it should be local.
There was a problem hiding this comment.
I make an experiment.
public class Test {
private static final int CLZ_FIELD = 65535;
public static void main(String[] args) {
final int localVariable = 65536;
System.out.println(localVariable);
System.out.println(CLZ_FIELD);
}
}
After decompilation， I got
public static void main(java.lang.String[]);
Code:
0: ldc #2 // int 65536
2: istore_1
3: getstatic #3 // Field java/lang/System.out:Ljava/io/PrintStream;
6: ldc #2 // int 65536
8: invokevirtual #4 // Method java/io/PrintStream.println:(I)V
11: getstatic #3 // Field java/lang/System.out:Ljava/io/PrintStream;
14: ldc #6 // int 65535
16: invokevirtual #4 // Method java/io/PrintStream.println:(I)V
19: return
Every time the local constant need one more step ('0: ldc #2') and one more stack space than static constant.
So you are right, I will make it static.
Fixes #3625
What is the purpose of the change
Remove magic number 5 in line 70 of org.apache.dubbo.configcenter.support.zookeeper.CacheListener#childEvent.
Brief changelog
Add constant 'MIN_PATH_ARRAY_LENGTH'
Verifying this change
XXXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
[Dubbo-XXX] Fix UnknownException when host config not exist #XXX. Each commit in the pull request should have a meaningful subject line and body.mvn clean install -DskipTests=false&mvn clean test-compile failsafe:integration-testto make sure unit-test and integration-test pass.