You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/01/24 13:17:49 UTC
[GitHub] [rocketmq] Kvicii opened a new pull request #3793: [ISSUE #3785] MQClientAPIImpl improvement
Kvicii opened a new pull request #3793:
URL: https://github.com/apache/rocketmq/pull/3793
For this class, I see a lot of switch...case blocks, and it's not necessary.Code clutter caused by excessive nesting.
So I merged all the code,but here is another problem,exceptions thrown in switch...case are not the same, so I think if further abstraction is needed.
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] Kvicii commented on pull request #3793: [ISSUE #3785] MQClientAPIImpl improvement
Posted by GitBox <gi...@apache.org>.
Kvicii commented on pull request #3793:
URL: https://github.com/apache/rocketmq/pull/3793#issuecomment-1024264188
@dongeforever
thx
First of all, I think that too many cases cause the code to be unreadable and confusing. If only one of the states needs to be judged, I think redundant judgments can be avoided.
Secondly, is the exception design only for client-related APIs?
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] coveralls commented on pull request #3793: [ISSUE #3785] MQClientAPIImpl improvement
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3793:
URL: https://github.com/apache/rocketmq/pull/3793#issuecomment-1020661443
[](https://coveralls.io/builds/45903154)
Coverage increased (+0.02%) to 51.129% when pulling **4766362fb1dc21f8477a2636fbcb4dc795e518e3 on Kvicii:issue_3785** into **8fdfb42ac8ceea24c7c1b6352df59ff8b585acbb on apache:develop**.
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] coveralls commented on pull request #3793: [ISSUE #3785] MQClientAPIImpl improvement
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3793:
URL: https://github.com/apache/rocketmq/pull/3793#issuecomment-1020661443
[](https://coveralls.io/builds/45903154)
Coverage increased (+0.02%) to 51.129% when pulling **4766362fb1dc21f8477a2636fbcb4dc795e518e3 on Kvicii:issue_3785** into **8fdfb42ac8ceea24c7c1b6352df59ff8b585acbb on apache:develop**.
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] Kvicii commented on pull request #3793: [ISSUE #3785] MQClientAPIImpl improvement
Posted by GitBox <gi...@apache.org>.
Kvicii commented on pull request #3793:
URL: https://github.com/apache/rocketmq/pull/3793#issuecomment-1030594463
Of course.
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] dongeforever commented on pull request #3793: [ISSUE #3785] MQClientAPIImpl improvement
Posted by GitBox <gi...@apache.org>.
dongeforever commented on pull request #3793:
URL: https://github.com/apache/rocketmq/pull/3793#issuecomment-1024190310
@Kvicii It is better to keep the same code style in MQClientAPIImpl.
Of course, the switch style has disadvantages such as excessive nesting, but it also has advantages. For the RPC layer, the switch style is more readable to handle different kinds of response code and easy to extend further codes.
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] dongeforever commented on pull request #3793: [ISSUE #3785] MQClientAPIImpl improvement
Posted by GitBox <gi...@apache.org>.
dongeforever commented on pull request #3793:
URL: https://github.com/apache/rocketmq/pull/3793#issuecomment-1024192465
@Kvicii The exceptions are a true problem.
It is better to provide a unified exception definition for all the APIs.
If you are interested, you could collect all the exception types, and introduce a new definition.
The HTTP error code could be a reference, 4xx for client error, 5xx for broker error.
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] dongeforever commented on pull request #3793: [ISSUE #3785] MQClientAPIImpl improvement
Posted by GitBox <gi...@apache.org>.
dongeforever commented on pull request #3793:
URL: https://github.com/apache/rocketmq/pull/3793#issuecomment-1030590323
@Kvicii Currently the community is developing the 5.0. Changing the basic exception mechanism may cause bugs and conflicts. It's better to hold on for the moment.
How about involving in the 5.0 rips(if it is at your convenience)
https://github.com/apache/rocketmq/blob/5.0.0-alpha/docs/cn/statictopic/RocketMQ_Static_Topic_Logic_Queue_%E8%AE%BE%E8%AE%A1.md
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] Kvicii commented on pull request #3793: [ISSUE #3785] MQClientAPIImpl improvement
Posted by GitBox <gi...@apache.org>.
Kvicii commented on pull request #3793:
URL: https://github.com/apache/rocketmq/pull/3793#issuecomment-1020107048
@duhenglucky hi, I modify this class, As I marked in the commit, do we need to further abstract a top-level exception class and a general checkXXX method
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] Kvicii commented on pull request #3793: [ISSUE #3785] MQClientAPIImpl improvement
Posted by GitBox <gi...@apache.org>.
Kvicii commented on pull request #3793:
URL: https://github.com/apache/rocketmq/pull/3793#issuecomment-1022881094
Can anyone help me review the code?
thx
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org