You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2021/07/12 05:01:58 UTC
[GitHub] [dubbo] JdjzV opened a new issue #8270: Optimize unstandard code
JdjzV opened a new issue #8270:
URL: https://github.com/apache/dubbo/issues/8270
Hi Dubbo,
When reading the code of the ConfigManager, I find some nonstandard code can be optimized.
Eg.
1. Inconsistent try catch block
The code in ConfigManager line 565.
```java
private <V> V write(Callable<V> callable) {
V value = null;
Lock writeLock = lock.writeLock();
try {
writeLock.lock();
value = callable.call();
} catch (RuntimeException e) {
throw e;
} catch (Throwable e) {
throw new RuntimeException(e.getCause());
} finally {
writeLock.unlock();
}
return value;
}
private <V> V read(Callable<V> callable) {
Lock readLock = lock.readLock();
V value = null;
try {
readLock.lock();
value = callable.call();
} catch (Throwable e) {
throw new RuntimeException(e);
} finally {
readLock.unlock();
}
return value;
}
```
For my perspective, the try-catch blocks in these two methods is used to avoiding dealing with exception in other callers.
So we needn't use different catch block to handle RuntimeException and Throwable in method write. Try-catch block in
method write can corresponding to method read.
In addition, the initial value ”null“ is unnecessary.
```java
V value = null;
```
can be simplified to
```java
V value;
```
2. Indentation
The code in ConfigManager line 720.
```java
String key = getId(config);
if (key == null) {
// generate key for non-default config compatible with API usages
key = generateConfigId(config);
}
```
The Indentation of code that after first line should be same as first line.
3. Useless else
The code in ConfigManager line 728.
```java
if (isEquals(existedConfig, config)) {
String type = config.getClass().getSimpleName();
throw new IllegalStateException(String.format("Duplicate %s found, there already has one default %s or more than two %ss have the same id, " +
"you can try to give each %s a different id, key: %s, prev: %s, new: %s", type, type, type, type, key, existedConfig, config));
} else {
configsMap.put(key, config);
}
```
Because there will throw Exception in if block, so else block is useless.
If the suggestions above I commit have problems, please let me know. Thank you!
Looking forward to your reply.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] ouyangnengda commented on issue #8270: Optimize unstandard code
Posted by GitBox <gi...@apache.org>.
ouyangnengda commented on issue #8270:
URL: https://github.com/apache/dubbo/issues/8270#issuecomment-878319722
你的这些修改我认为可有可无的，原来的写法并没有任何问题，只是不同人的编码习惯不同罢了。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo] xiaoheng1 closed issue #8270: Optimize unstandard code
Posted by GitBox <gi...@apache.org>.
xiaoheng1 closed issue #8270:
URL: https://github.com/apache/dubbo/issues/8270
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org