You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "redoom (via GitHub)" <gi...@apache.org> on 2025/11/11 04:52:45 UTC
[PR] refactor: Avoid calling overridable methods in constructors [dubbo]
redoom opened a new pull request, #15767:
URL: https://github.com/apache/dubbo/pull/15767
## Motivation
https://github.com/apache/dubbo/issues/15600#event-20779028745
## The Problem
```
public abstract class AbstractServer {
public AbstractServer(URL url, ChannelHandler handler) {
super(url, handler);
doOpen();
}
protected abstract void doOpen() throws Throwable;
}
public class NettyServer extends AbstractServer {
private ServerBootstrap bootstrap; // Still null when doOpen() called
@Override
protected void doOpen() {
bootstrap.group(...);
}
}
```
## Solution
Two-phase initialization with static factory methods:
```
// ✅ After: Separate construction from initialization
protected AbstractServer(URL url, ChannelHandler handler) {
super(url, handler);
// Only field initialization
}
protected final void init() throws RemotingException {
doOpen(); // object fully constructed
}
public static NettyServer create(URL url, ChannelHandler handler) {
NettyServer server = new NettyServer(url, handler);
server.init();
return server;
}
private NettyServer(URL url, ChannelHandler handler) {
super(url, ChannelHandlers.wrap(handler, url));
}
```
## Changes
- Split initialization: construction (fields only) + init() (resources)
- Private constructors + static factory methods
- Final init() methods prevent subclass override
```
// ✅
NettyServer server = NettyServer.create(url, handler);
// ❌
// NettyServer server = new NettyServer(url, handler);
```
--
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
Re: [PR] refactor: Avoid calling overridable methods in constructors [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on code in PR #15767:
URL: https://github.com/apache/dubbo/pull/15767#discussion_r2513416270
##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/affinity/AffinityStateRouter.java:
##########
@@ -80,7 +88,7 @@ public AffinityStateRouter(URL url) {
}
Review Comment:
```
if (this.enabled) {
this.init(affinityKey); <== called at consturction 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: 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
Re: [PR] refactor: Avoid calling overridable methods in constructors [dubbo]
Posted by "redoom (via GitHub)" <gi...@apache.org>.
redoom commented on code in PR #15767:
URL: https://github.com/apache/dubbo/pull/15767#discussion_r2534428435
##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/affinity/config/AffinityListenableStateRouter.java:
##########
@@ -53,15 +53,14 @@ public abstract class AffinityListenableStateRouter<T> extends AbstractStateRout
private volatile AffinityStateRouter<T> affinityRouter;
private final String ruleKey;
- public AffinityListenableStateRouter(URL url, String ruleKey) {
+ protected AffinityListenableStateRouter(URL url, String ruleKey) {
super(url);
this.setForce(false);
- this.init(ruleKey);
Review Comment:
I didn't quite get what you meant,AffinityServiceStateRouter.java is abstract class , so I put init in the subclasses instead.
> Maybe we don't need to modify init? Because you've already decorated it with the `final`.
--
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
Re: [PR] refactor: Avoid calling overridable methods in constructors [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on code in PR #15767:
URL: https://github.com/apache/dubbo/pull/15767#discussion_r2534369125
##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/affinity/config/AffinityListenableStateRouter.java:
##########
@@ -53,15 +53,14 @@ public abstract class AffinityListenableStateRouter<T> extends AbstractStateRout
private volatile AffinityStateRouter<T> affinityRouter;
private final String ruleKey;
- public AffinityListenableStateRouter(URL url, String ruleKey) {
+ protected AffinityListenableStateRouter(URL url, String ruleKey) {
super(url);
this.setForce(false);
- this.init(ruleKey);
Review Comment:
Maybe we don't need to modify init? Because you've already decorated it with the `final`.
--
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
Re: [PR] refactor: Avoid calling overridable methods in constructors [dubbo]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #15767:
URL: https://github.com/apache/dubbo/pull/15767#issuecomment-3515035455
## [Codecov](https://app.codecov.io/gh/apache/dubbo/pull/15767?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `53.16456%` with `37 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 32.11%. Comparing base ([`7cee17d`](https://app.codecov.io/gh/apache/dubbo/commit/7cee17d0a07aeae444f67445e76a469feadfffcb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`a3ad268`](https://app.codecov.io/gh/apache/dubbo/commit/a3ad2687539b7ace5f1d7fdb53b60050bba2080f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/dubbo/pull/15767?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...e/dubbo/registry/integration/RegistryProtocol.java](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&filepath=dubbo-registry%2Fdubbo-registry-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fregistry%2Fintegration%2FRegistryProtocol.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9pbnRlZ3JhdGlvbi9SZWdpc3RyeVByb3RvY29sLmphdmE=) | 60.00% | [2 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...pache/dubbo/remoting/transport/AbstractClient.java](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&filepath=dubbo-remoting%2Fdubbo-remoting-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fremoting%2Ftransport%2FAbstractClient.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvQWJzdHJhY3RDbGllbnQuamF2YQ==) | 20.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...affinity/config/AffinityListenableStateRouter.java](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&filepath=dubbo-cluster%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Frpc%2Fcluster%2Frouter%2Faffinity%2Fconfig%2FAffinityListenableStateRouter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvcm91dGVyL2FmZmluaXR5L2NvbmZpZy9BZmZpbml0eUxpc3RlbmFibGVTdGF0ZVJvdXRlci5qYXZh) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...bo/remoting/transport/netty4/NettyHttp3Server.java](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&filepath=dubbo-remoting%2Fdubbo-remoting-http3%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fremoting%2Ftransport%2Fnetty4%2FNettyHttp3Server.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctaHR0cDMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlbW90aW5nL3RyYW5zcG9ydC9uZXR0eTQvTmV0dHlIdHRwM1NlcnZlci5qYXZh) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...he/dubbo/remoting/transport/netty/NettyClient.java](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&filepath=dubbo-remoting%2Fdubbo-remoting-netty%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fremoting%2Ftransport%2Fnetty%2FNettyClient.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlbW90aW5nL3RyYW5zcG9ydC9uZXR0eS9OZXR0eUNsaWVudC5qYXZh) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ng/transport/netty/NettyPortUnificationServer.java](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&filepath=dubbo-remoting%2Fdubbo-remoting-netty%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fremoting%2Ftransport%2Fnetty%2FNettyPortUnificationServer.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlbW90aW5nL3RyYW5zcG9ydC9uZXR0eS9OZXR0eVBvcnRVbmlmaWNhdGlvblNlcnZlci5qYXZh) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...he/dubbo/remoting/transport/netty/NettyServer.java](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&filepath=dubbo-remoting%2Fdubbo-remoting-netty%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fremoting%2Ftransport%2Fnetty%2FNettyServer.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlbW90aW5nL3RyYW5zcG9ydC9uZXR0eS9OZXR0eVNlcnZlci5qYXZh) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...g/transport/netty4/NettyPortUnificationServer.java](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&filepath=dubbo-remoting%2Fdubbo-remoting-netty4%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fremoting%2Ftransport%2Fnetty4%2FNettyPortUnificationServer.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHk0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvbmV0dHk0L05ldHR5UG9ydFVuaWZpY2F0aW9uU2VydmVyLmphdmE=) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...c/cluster/router/affinity/AffinityStateRouter.java](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&filepath=dubbo-cluster%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Frpc%2Fcluster%2Frouter%2Faffinity%2FAffinityStateRouter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvcm91dGVyL2FmZmluaXR5L0FmZmluaXR5U3RhdGVSb3V0ZXIuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...bbo/remoting/transport/netty/NettyTransporter.java](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&filepath=dubbo-remoting%2Fdubbo-remoting-netty%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fremoting%2Ftransport%2Fnetty%2FNettyTransporter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlbW90aW5nL3RyYW5zcG9ydC9uZXR0eS9OZXR0eVRyYW5zcG9ydGVyLmphdmE=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| ... and [7 more](https://app.codecov.io/gh/apache/dubbo/pull/15767?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
> :exclamation: There is a different number of reports uploaded between BASE (7cee17d) and HEAD (a3ad268). Click for more details.
>
> <details><summary>HEAD has 7 uploads less than BASE</summary>
>
>| Flag | BASE (7cee17d) | HEAD (a3ad268) |
>|------|------|------|
>|integration-tests-java8|1|0|
>|integration-tests-java21|1|0|
>|unit-tests-java8|1|0|
>|unit-tests-java11|1|0|
>|unit-tests-java17|1|0|
>|unit-tests-java21|1|0|
>|unit-tests-java25|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 3.3 #15767 +/- ##
=============================================
- Coverage 60.78% 32.11% -28.68%
+ Complexity 11702 10138 -1564
=============================================
Files 1938 1937 -1
Lines 88640 88658 +18
Branches 13379 13296 -83
=============================================
- Hits 53883 28470 -25413
- Misses 29239 56033 +26794
+ Partials 5518 4155 -1363
```
| [Flag](https://app.codecov.io/gh/apache/dubbo/pull/15767/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/15767/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/15767/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [samples-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/15767/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `32.06% <53.16%> (+0.03%)` | :arrow_up: |
| [samples-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/15767/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `29.71% <48.10%> (-0.02%)` | :arrow_down: |
| [unit-tests-java11](https://app.codecov.io/gh/apache/dubbo/pull/15767/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java17](https://app.codecov.io/gh/apache/dubbo/pull/15767/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/15767/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java25](https://app.codecov.io/gh/apache/dubbo/pull/15767/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/15767/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/dubbo/pull/15767?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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
Re: [PR] refactor: Avoid calling overridable methods in constructors [dubbo]
Posted by "redoom (via GitHub)" <gi...@apache.org>.
redoom commented on code in PR #15767:
URL: https://github.com/apache/dubbo/pull/15767#discussion_r2534298087
##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/affinity/config/AffinityListenableStateRouter.java:
##########
@@ -53,15 +53,14 @@ public abstract class AffinityListenableStateRouter<T> extends AbstractStateRout
private volatile AffinityStateRouter<T> affinityRouter;
private final String ruleKey;
- public AffinityListenableStateRouter(URL url, String ruleKey) {
+ protected AffinityListenableStateRouter(URL url, String ruleKey) {
super(url);
this.setForce(false);
- this.init(ruleKey);
Review Comment:
It's an abstract class, so I didn't make a factory.
--
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
Re: [PR] refactor: Avoid calling overridable methods in constructors [dubbo]
Posted by "redoom (via GitHub)" <gi...@apache.org>.
redoom commented on code in PR #15767:
URL: https://github.com/apache/dubbo/pull/15767#discussion_r2534276938
##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/affinity/config/AffinityListenableStateRouter.java:
##########
@@ -85,8 +84,12 @@ public synchronized void process(ConfigChangedEvent event) {
e);
}
}
+
+ onConfigChange(event);
Review Comment:
I left an extension point because I wasn't sure if flexibility would be needed after making the method final. Would you prefer I remove it？
--
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
Re: [PR] refactor: Avoid calling overridable methods in constructors [dubbo]
Posted by "redoom (via GitHub)" <gi...@apache.org>.
redoom commented on PR #15767:
URL: https://github.com/apache/dubbo/pull/15767#issuecomment-3542567752
> I think we shouldn’t initialize objects in their subclasses. The better way, like the code you modified, is to use final or private to restrict them so they can only be initialized within their own class.
Got it
--
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
Re: [PR] refactor: Avoid calling overridable methods in constructors [dubbo]
Posted by "redoom (via GitHub)" <gi...@apache.org>.
redoom commented on code in PR #15767:
URL: https://github.com/apache/dubbo/pull/15767#discussion_r2534261819
##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/affinity/config/AffinityListenableStateRouter.java:
##########
@@ -53,15 +53,14 @@ public abstract class AffinityListenableStateRouter<T> extends AbstractStateRout
private volatile AffinityStateRouter<T> affinityRouter;
private final String ruleKey;
- public AffinityListenableStateRouter(URL url, String ruleKey) {
+ protected AffinityListenableStateRouter(URL url, String ruleKey) {
super(url);
this.setForce(false);
- this.init(ruleKey);
Review Comment:
It's an abstract class, so I didn't make a factory. I thought about adding an abstract method back then, but I'll make it a concrete method now.
--
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
Re: [PR] refactor: Avoid calling overridable methods in constructors [dubbo]
Posted by "redoom (via GitHub)" <gi...@apache.org>.
redoom commented on code in PR #15767:
URL: https://github.com/apache/dubbo/pull/15767#discussion_r2534261819
##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/affinity/config/AffinityListenableStateRouter.java:
##########
@@ -53,15 +53,14 @@ public abstract class AffinityListenableStateRouter<T> extends AbstractStateRout
private volatile AffinityStateRouter<T> affinityRouter;
private final String ruleKey;
- public AffinityListenableStateRouter(URL url, String ruleKey) {
+ protected AffinityListenableStateRouter(URL url, String ruleKey) {
super(url);
this.setForce(false);
- this.init(ruleKey);
Review Comment:
It's an abstract class, so I didn't make a factory. I thought about adding an abstract method back then, but I'll make it a concrete method now.
--
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
Re: [PR] refactor: Avoid calling overridable methods in constructors [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on code in PR #15767:
URL: https://github.com/apache/dubbo/pull/15767#discussion_r2534464030
##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/affinity/config/AffinityListenableStateRouter.java:
##########
@@ -53,15 +53,14 @@ public abstract class AffinityListenableStateRouter<T> extends AbstractStateRout
private volatile AffinityStateRouter<T> affinityRouter;
private final String ruleKey;
- public AffinityListenableStateRouter(URL url, String ruleKey) {
+ protected AffinityListenableStateRouter(URL url, String ruleKey) {
super(url);
this.setForce(false);
- this.init(ruleKey);
Review Comment:
I mean I don't know why it's better to put init in the subclasses. Because you've marked init as final.
--
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
Re: [PR] refactor: Avoid calling overridable methods in constructors [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on code in PR #15767:
URL: https://github.com/apache/dubbo/pull/15767#discussion_r2534133286
##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/affinity/config/AffinityListenableStateRouter.java:
##########
@@ -53,15 +53,14 @@ public abstract class AffinityListenableStateRouter<T> extends AbstractStateRout
private volatile AffinityStateRouter<T> affinityRouter;
private final String ruleKey;
- public AffinityListenableStateRouter(URL url, String ruleKey) {
+ protected AffinityListenableStateRouter(URL url, String ruleKey) {
super(url);
this.setForce(false);
- this.init(ruleKey);
Review Comment:
why not use the same style like AffinityStateRouter.java, I don't think init at AffinityServiceStateRouter is a better way....
##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/affinity/config/AffinityListenableStateRouter.java:
##########
@@ -85,8 +84,12 @@ public synchronized void process(ConfigChangedEvent event) {
e);
}
}
+
+ onConfigChange(event);
Review Comment:
why?
--
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
Re: [PR] refactor: Avoid calling overridable methods in constructors [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on code in PR #15767:
URL: https://github.com/apache/dubbo/pull/15767#discussion_r2534354765
##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/affinity/config/AffinityListenableStateRouter.java:
##########
@@ -85,8 +84,12 @@ public synchronized void process(ConfigChangedEvent event) {
e);
}
}
+
+ onConfigChange(event);
Review Comment:
yes. Now I don't think we need this.
--
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
Re: [PR] refactor: Avoid calling overridable methods in constructors [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on PR #15767:
URL: https://github.com/apache/dubbo/pull/15767#issuecomment-3542509049
I think we shouldn’t initialize objects in their subclasses. The better way, like the code you modified, is to use final or private to restrict them so they can only be initialized within their own class.
--
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