You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@eventmesh.apache.org by "PickBas (via GitHub)" <gi...@apache.org> on 2023/03/29 07:32:38 UTC

[GitHub] [eventmesh] PickBas opened a new pull request, #3551: [ISSUE #3506] removed return statement from the for loop

PickBas opened a new pull request, #3551:
URL: https://github.com/apache/eventmesh/pull/3551

   Fixes #3506.
   
   ### Modifications
   
   Removed return statement from the for loop
   
   ### Documentation
   
   - Does this pull request introduce a new feature? (no)
   


-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org


[GitHub] [eventmesh] mytang0 commented on pull request #3551: [ISSUE #3506] removed return statement from the for loop

Posted by "mytang0 (via GitHub)" <gi...@apache.org>.
mytang0 commented on PR #3551:
URL: https://github.com/apache/eventmesh/pull/3551#issuecomment-1489804749

   > @mytang0 Are changes still requested or it's ok?
   
   Hello @PickBas , can you help to optimize it? Remove the for loop and add a comment. Grateful.


-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


[GitHub] [eventmesh] PickBas commented on a diff in pull request #3551: [ISSUE #3506] removed return statement from the for loop

Posted by "PickBas (via GitHub)" <gi...@apache.org>.
PickBas commented on code in PR #3551:
URL: https://github.com/apache/eventmesh/pull/3551#discussion_r1152865610


##########
eventmesh-registry-plugin/eventmesh-registry-etcd/src/main/java/org/apache/eventmesh/registry/etcd/service/EtcdCustomService.java:
##########
@@ -77,31 +77,29 @@ public List<EventMeshServicePubTopicInfo> findEventMeshServicePubTopicInfos() th
 
     @Nullable
     public EventMeshAppSubTopicInfo findEventMeshAppSubTopicInfoByGroup(String group) throws RegistryException {
-
         Client client = getEtcdClient();
         String keyPrefix = KEY_PREFIX + KEY_APP + EtcdConstant.KEY_SEPARATOR + group;
         List<KeyValue> keyValues = null;
         try {
             ByteSequence keyByteSequence = ByteSequence.from(keyPrefix.getBytes(Constants.DEFAULT_CHARSET));
-
             GetOption getOption = GetOption.newBuilder().withPrefix(keyByteSequence).build();
-
             keyValues = client.getKVClient().get(keyByteSequence, getOption).get().getKvs();
-
-
             if (CollectionUtils.isNotEmpty(keyValues)) {
-                for (KeyValue kv : keyValues) {
-                    EventMeshAppSubTopicInfo eventMeshAppSubTopicInfo =
-                        JsonUtils.parseObject(new String(kv.getValue().getBytes(), Constants.DEFAULT_CHARSET), EventMeshAppSubTopicInfo.class);
+                EventMeshAppSubTopicInfo eventMeshAppSubTopicInfo =
+                    JsonUtils.parseObject(
+                        new String(keyValues.get(0).getValue().getBytes(), Constants.DEFAULT_CHARSET),
+                        EventMeshAppSubTopicInfo.class
+                    );
+                if (eventMeshAppSubTopicInfo != null) {
                     return eventMeshAppSubTopicInfo;
+                } else {
+                    throw new NullPointerException("eventMeshAppSubTopicInfo is null!");

Review Comment:
   Done



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


[GitHub] [eventmesh] PickBas commented on pull request #3551: [ISSUE #3506] removed return statement from the for loop

Posted by "PickBas (via GitHub)" <gi...@apache.org>.
PickBas commented on PR #3551:
URL: https://github.com/apache/eventmesh/pull/3551#issuecomment-1489797013

   @mytang0 Are changes still requested or it's ok?


-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


[GitHub] [eventmesh] PickBas commented on pull request #3551: [ISSUE #3506] removed return statement from the for loop

Posted by "PickBas (via GitHub)" <gi...@apache.org>.
PickBas commented on PR #3551:
URL: https://github.com/apache/eventmesh/pull/3551#issuecomment-1489838248

   @mytang0 have a look please


-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


[GitHub] [eventmesh] mytang0 merged pull request #3551: [ISSUE #3506] removed return statement from the for loop

Posted by "mytang0 (via GitHub)" <gi...@apache.org>.
mytang0 merged PR #3551:
URL: https://github.com/apache/eventmesh/pull/3551


-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org


[GitHub] [eventmesh] mytang0 commented on a diff in pull request #3551: [ISSUE #3506] removed return statement from the for loop

Posted by "mytang0 (via GitHub)" <gi...@apache.org>.
mytang0 commented on code in PR #3551:
URL: https://github.com/apache/eventmesh/pull/3551#discussion_r1152861831


##########
eventmesh-registry-plugin/eventmesh-registry-etcd/src/main/java/org/apache/eventmesh/registry/etcd/service/EtcdCustomService.java:
##########
@@ -77,31 +77,29 @@ public List<EventMeshServicePubTopicInfo> findEventMeshServicePubTopicInfos() th
 
     @Nullable
     public EventMeshAppSubTopicInfo findEventMeshAppSubTopicInfoByGroup(String group) throws RegistryException {
-
         Client client = getEtcdClient();
         String keyPrefix = KEY_PREFIX + KEY_APP + EtcdConstant.KEY_SEPARATOR + group;
         List<KeyValue> keyValues = null;
         try {
             ByteSequence keyByteSequence = ByteSequence.from(keyPrefix.getBytes(Constants.DEFAULT_CHARSET));
-
             GetOption getOption = GetOption.newBuilder().withPrefix(keyByteSequence).build();
-
             keyValues = client.getKVClient().get(keyByteSequence, getOption).get().getKvs();
-
-
             if (CollectionUtils.isNotEmpty(keyValues)) {
-                for (KeyValue kv : keyValues) {
-                    EventMeshAppSubTopicInfo eventMeshAppSubTopicInfo =
-                        JsonUtils.parseObject(new String(kv.getValue().getBytes(), Constants.DEFAULT_CHARSET), EventMeshAppSubTopicInfo.class);
+                EventMeshAppSubTopicInfo eventMeshAppSubTopicInfo =
+                    JsonUtils.parseObject(
+                        new String(keyValues.get(0).getValue().getBytes(), Constants.DEFAULT_CHARSET),
+                        EventMeshAppSubTopicInfo.class
+                    );
+                if (eventMeshAppSubTopicInfo != null) {
                     return eventMeshAppSubTopicInfo;
+                } else {
+                    throw new NullPointerException("eventMeshAppSubTopicInfo is null!");

Review Comment:
   Return directly, there is already a null check outside.



-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


[GitHub] [eventmesh] xwm1992 commented on pull request #3551: [ISSUE #3506] removed return statement from the for loop

Posted by "xwm1992 (via GitHub)" <gi...@apache.org>.
xwm1992 commented on PR #3551:
URL: https://github.com/apache/eventmesh/pull/3551#issuecomment-1489761652

   ![52148992-72b1-421a-9463-1d23d9c975ed](https://user-images.githubusercontent.com/13237619/228747734-8c096de8-fea8-462a-bb19-febe7139b978.png)
   I haven confirmed with the author, for now there is only one result under the loop, it's ok.


-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org


[GitHub] [eventmesh] mytang0 commented on pull request #3551: [ISSUE #3506] removed return statement from the for loop

Posted by "mytang0 (via GitHub)" <gi...@apache.org>.
mytang0 commented on PR #3551:
URL: https://github.com/apache/eventmesh/pull/3551#issuecomment-1489791317

   > ![52148992-72b1-421a-9463-1d23d9c975ed](https://user-images.githubusercontent.com/13237619/228747734-8c096de8-fea8-462a-bb19-febe7139b978.png) I haven confirmed with the author, for now there is only one result under the loop, it's ok.
   
   Ok, so the result is correct. But the code looks a bit weird.


-- 
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: issues-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org