dubbo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [dubbo] JdjzV opened a new issue #8270: Optimize unstandard code
Date Mon, 12 Jul 2021 05:01:58 GMT

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


Mime
View raw message