XComp commented on code in PR #22959: URL: https://github.com/apache/flink/pull/22959#discussion_r1254068037
########## flink-runtime/src/main/java/org/apache/flink/runtime/leaderelection/LeaderElectionService.java: ########## Review Comment: I'm not sure whether we want to expose these methods through the interface. The purpose of the `LeaderElectionService` interface is to serve in the `AbstractHaServices`. The `AbstractLeaderElectionService` methods do not need to be visible there. Instead, they serve as an interface to the `DefaultLeaderElectionService`. Hence, we could move them into a separate interface (maybe an inner interface `DefaultLeaderElection.ParentService`). `DefaultLeaderElection` is a package-private class. That way, we wouldn't need to expose the corresponding interface methods outside of the package. The `DefaultLeaderElectionService` could still implement the interface because it's located in the same package. The problem with the above-mentioned approach is that we're still exposing the interface methods in `DefaultLeaderElectionService` because interface methods cannot be package-private. An alternative solution is to go ahead with the previously mentioned approach but implement `DefaultLeaderElection.ParentService` as an `package-private abstract` class (instead of an interface) with all the methods being `package-private abstract` methods. `DefaultLeaderElectionService` could extend this abstract class. It serves as some kind of mixin here. Generally, interfaces are the way to go in Java - but the abstract class would work here as well because `DefaultLeaderElectionService` doesn't have any parent class. WDYT? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
