lindong28 commented on PR #21690: URL: https://github.com/apache/flink/pull/21690#issuecomment-1401300664
> So that's not true as well. Regardless how you implement it, the caller has to care about how many times the DataOutput#emitRecord() can be called in a row. If you hide this logic in a method/interface that passed separately, it doesn't make it disconnected in any way. Instead of passing one interface with both emitRecord() and something that returns canEmitBatchOfRecords() (either via return value or separate method in the DataOutput interface), you have to pass two interfaces. One that controls how the other should be used. I don't see any benefit of this approach: @pnowojski It seems that we are talking about different things here. I think Rui is saying that "callee should not care about the caller". And you are saying that "caller has to care about how many times callee can be called". IMO both statements are correct. This is the main reason I am against the method `boolean DataOutput#emitRecord` -- it breaks the commonly accepted principle that "the definition of interface method should be self-descriptive and not depend on what caller does". If an interface method does not follow this principle, IMO it pretty much degrades to a class as it introduces bi-directional ties between caller and callee. I also asked @becketqin offline to comment on this point but I am not sure if he has time. Anyway, in case we can not agree on this point anytime soon, do you mind if we create a new PR to fix the bug and let this PR focus on the refactoring? -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
