rkhachatryan commented on issue #11403: [FLINK-16316][operators] Implement new StreamOperatorBase as a replacement for AbstractStreamOperator URL: https://github.com/apache/flink/pull/11403#issuecomment-600368696 > 1. StreamOperatorStateHandler should be not a field inside StreamOperatorBase, but a wrapper around it, but... > 1. AbstractStreamOperator (and hence I assume StreamOperatorBase as well), needs to expose things from within StreamOperatorStateHandler. > 1. Purpose of AbstractStreamOperator is to provide a convenient class, so that users do not have to implement all of the methods manually. > putting those three together, it's hard for me to imagine a solution, where we split StreamOperatorBase I think an important point is missed from my comment: we should change the **interfaces** first (i.e. `StreamOperator`, not `StreamOperatorBase`). Then much of the need for convenience will go away as well as difficulty of splitting the implementation. > without multiple inheritance it's impossible to provide abstract classes for combination ... I think we can use what is called traits (or mixins) in other languages and can be achieved with default interface methods in Java. > My purpose here was to solve some of the problems that I was aware before (non final transient methods, and incompatibility with MultipleInputStreamOperator interface). To me these are two separate things: 1. providing a base class for `MultipleInputStreamOperator`; for this, if we can't use existing abstraction then why not just copy what we need from `AbstractStreamOperator`? IMO, code duplication in one place is much lesser evil then a wrong abstraction for hundred classes 1. fixing issues in `AbstractStreamOperator`; I totally agree that we need a separate FLIP and a much broader discussion/consideration if we want it to become base class for all operators > Does it mean that you are generally speaking fine with proceeding with the PR as it is? ... I don't think we should replace the `AbstractStreamOperator` now. I'd rather: 1. have a new base class just for the `MultipleInputStreamOperator` case 1. proceed with the refactorings *inside* `AbstractStreamOperator` - but not deprecate it Then, after we get an understanding of how the top-level interface should look and after porting some operators on to new base class we can think of their generalization.
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
