Hi everyone,
I'm currently thinking about how to implement FLINK-8606. The reason behind it is that Java users are able to see all variables and methods that are declared 'private[flink]' or even 'protected' in Scala. Classes such as TableEnvironment look very messy from the outside in Java. Since we cannot change the visibility of Scala protected members, I was thinking about a bigger change to solve this issue once and for all. My idea is to convert all TableEnvironment classes and maybe the Table class into traits. The actual implementation would end up in some internal classes such as "InternalTableEnvironment" that implement the public traits. The goal would be to stay source code compatible. What do you think? Regards, Timo |
Hi Timo,
Thanks for looking into this Timo. It's becoming increasingly messy for my trying to locate the correct functions in IDE :-/ This is probably due to the fact that Scala and Java access modifiers / qualifiers are subtly and fundamentally different. Using Trait might be the best solution here. Another way I can think of is to move the all TableEnvironment classes to Java side, but that would probably introduce a lot of issue we need to resolve on the Scala side though. "protected" is less restrictive in Java but there's really no equivalent of package private modifier on Java. I was wondering is there any better way to provide backward-compatible support though. I played around with it and seems like every "protected" field will create a private Java member and a public getter, should we add them all and annotate with "@Deprecated" ? -- Rong On Thu, Mar 1, 2018 at 10:58 AM, Timo Walther <[hidden email]> wrote: > Hi everyone, > > I'm currently thinking about how to implement FLINK-8606. The reason > behind it is that Java users are able to see all variables and methods that > are declared 'private[flink]' or even 'protected' in Scala. Classes such as > TableEnvironment look very messy from the outside in Java. Since we cannot > change the visibility of Scala protected members, I was thinking about a > bigger change to solve this issue once and for all. My idea is to convert > all TableEnvironment classes and maybe the Table class into traits. The > actual implementation would end up in some internal classes such as > "InternalTableEnvironment" that implement the public traits. The goal would > be to stay source code compatible. > > What do you think? > > Regards, > Timo > > |
Hi Timo,
I am throwing some second thoughts here, as I don't quite see what trait provides over abstract class here for TableEnvironment case. Trait in scala can also have implementation and you can have 'private[flink]' or 'protected' type and method in trait as well. AFAIK, the differences between Scala trait and abstract class are: 1) you can have constructor for abstract class, but not in trait 2) Abstract classes are fully interoperable with Java. You can call them from Java code without any wrappers. Traits are fully interoperable only if they do not contain any implementation code for scala 2.11. 3) you can do multiple inheritance or mixin composition with trait. In the TableEnvironment case, 1) I don't see a need for mixin, and class hierarchy seems fit better here by design. 2) to better interoperate with Java from scala 2.11, it's better to use abstract class. (But AFAIK, scala 2.12 and java 8 would be compatible, though) 3) you might pay a bit performance overhead with trait (compiled to interface) compared to abstract class, but it's not a big deal here. But in other cases, trait might be a better one if it might be reused and mixined in multiple, unrelated classes. So another option would be to refactor TableEnvironment to clean up or move the 'private[flink]' or 'protected' stuff to the actual implementor (e.g. 'InternalTableEnvironment') as you would do for your trait approach for TableEnvironment. I think this option might help with backward compatibility as well. Thanks. Shuyi On Fri, Mar 2, 2018 at 10:25 AM, Rong Rong <[hidden email]> wrote: > Hi Timo, > > Thanks for looking into this Timo. It's becoming increasingly messy for my > trying to locate the correct functions in IDE :-/ > > This is probably due to the fact that Scala and Java access modifiers / > qualifiers are subtly and fundamentally different. Using Trait might be the > best solution here. Another way I can think of is to move the all > TableEnvironment classes to Java side, but that would probably introduce a > lot of issue we need to resolve on the Scala side though. "protected" is > less restrictive in Java but there's really no equivalent of package > private modifier on Java. > > I was wondering is there any better way to provide backward-compatible > support though. I played around with it and seems like every "protected" > field will create a private Java member and a public getter, should we add > them all and annotate with "@Deprecated" ? > -- > Rong > > On Thu, Mar 1, 2018 at 10:58 AM, Timo Walther <[hidden email]> wrote: > > > Hi everyone, > > > > I'm currently thinking about how to implement FLINK-8606. The reason > > behind it is that Java users are able to see all variables and methods > that > > are declared 'private[flink]' or even 'protected' in Scala. Classes such > as > > TableEnvironment look very messy from the outside in Java. Since we > cannot > > change the visibility of Scala protected members, I was thinking about a > > bigger change to solve this issue once and for all. My idea is to convert > > all TableEnvironment classes and maybe the Table class into traits. The > > actual implementation would end up in some internal classes such as > > "InternalTableEnvironment" that implement the public traits. The goal > would > > be to stay source code compatible. > > > > What do you think? > > > > Regards, > > Timo > > > > > -- "So you have to trust that the dots will somehow connect in your future." |
Hi Shuyi,
I am already assuming all package private and protected modifiers will be cleaned up and moved to internal implementation. Please correct me if I were wrong, Timo. Thanks, Rong On Fri, Mar 2, 2018, 5:02 PM Shuyi Chen <[hidden email]> wrote: > Hi Timo, > > I am throwing some second thoughts here, as I don't quite see what trait > provides over abstract class here for TableEnvironment case. Trait in scala > can also have implementation and you can have 'private[flink]' or > 'protected' type and method in trait as well. > > AFAIK, the differences between Scala trait and abstract class are: > 1) you can have constructor for abstract class, but not in trait > 2) Abstract classes are fully interoperable with Java. You can call them > from Java code without any wrappers. Traits are fully interoperable only if > they do not contain any implementation code for scala 2.11. > 3) you can do multiple inheritance or mixin composition with trait. > > In the TableEnvironment case, > 1) I don't see a need for mixin, and class hierarchy seems fit better here > by design. > 2) to better interoperate with Java from scala 2.11, it's better to use > abstract class. (But AFAIK, scala 2.12 and java 8 would be compatible, > though) > 3) you might pay a bit performance overhead with trait (compiled to > interface) compared to abstract class, but it's not a big deal here. > > But in other cases, trait might be a better one if it might be reused and > mixined in multiple, unrelated classes. > > So another option would be to refactor TableEnvironment to clean up or move > the 'private[flink]' or 'protected' stuff to the actual implementor > (e.g. 'InternalTableEnvironment') as > you would do for your trait approach for TableEnvironment. I think this > option might help with backward compatibility as well. Thanks. > > Shuyi > > On Fri, Mar 2, 2018 at 10:25 AM, Rong Rong <[hidden email]> wrote: > > > Hi Timo, > > > > Thanks for looking into this Timo. It's becoming increasingly messy for > my > > trying to locate the correct functions in IDE :-/ > > > > This is probably due to the fact that Scala and Java access modifiers / > > qualifiers are subtly and fundamentally different. Using Trait might be > the > > best solution here. Another way I can think of is to move the all > > TableEnvironment classes to Java side, but that would probably introduce > a > > lot of issue we need to resolve on the Scala side though. "protected" is > > less restrictive in Java but there's really no equivalent of package > > private modifier on Java. > > > > I was wondering is there any better way to provide backward-compatible > > support though. I played around with it and seems like every "protected" > > field will create a private Java member and a public getter, should we > add > > them all and annotate with "@Deprecated" ? > > -- > > Rong > > > > On Thu, Mar 1, 2018 at 10:58 AM, Timo Walther <[hidden email]> > wrote: > > > > > Hi everyone, > > > > > > I'm currently thinking about how to implement FLINK-8606. The reason > > > behind it is that Java users are able to see all variables and methods > > that > > > are declared 'private[flink]' or even 'protected' in Scala. Classes > such > > as > > > TableEnvironment look very messy from the outside in Java. Since we > > cannot > > > change the visibility of Scala protected members, I was thinking about > a > > > bigger change to solve this issue once and for all. My idea is to > convert > > > all TableEnvironment classes and maybe the Table class into traits. The > > > actual implementation would end up in some internal classes such as > > > "InternalTableEnvironment" that implement the public traits. The goal > > would > > > be to stay source code compatible. > > > > > > What do you think? > > > > > > Regards, > > > Timo > > > > > > > > > > > > -- > "So you have to trust that the dots will somehow connect in your future." > |
Hi everyone,
@Rong: Yes, that's what I meant. Package private and protected modifiers and fields will be moved to internal implementation. The hierachy would look like: trait TableEnvironment { def fromTableSource(source: TableSource[_]): Table def registerExternalCatalog(name: String, externalCatalog: ExternalCatalog): Unit ... } trait StreamTableEnvironment extends TableEnvironment { def fromDataStream[T](dataStream: DataStream[T]): Table ... } abstract class InternalTableEnvironment extends TableEnvironment { protected def getConversionMapper[OUT](...) = { implementation is here} ... } class InternalStreamTableEnvironment extends InternalTableEnvironment with StreamTableEnvironment { ... implementation is here } I think having Interface and InterfaceImpl is a typical pattern that could help in making the API more usable. Yes, implementing everything in Java would help but this would fragement the code base and would need a lot of work since the table environment implementation is quite large. Regards, Timo Am 3/3/18 um 8:07 AM schrieb Rong Rong: > Hi Shuyi, > > I am already assuming all package private and protected modifiers will be > cleaned up and moved to internal implementation. Please correct me if I > were wrong, Timo. > > Thanks, > Rong > > > On Fri, Mar 2, 2018, 5:02 PM Shuyi Chen <[hidden email]> wrote: > >> Hi Timo, >> >> I am throwing some second thoughts here, as I don't quite see what trait >> provides over abstract class here for TableEnvironment case. Trait in scala >> can also have implementation and you can have 'private[flink]' or >> 'protected' type and method in trait as well. >> >> AFAIK, the differences between Scala trait and abstract class are: >> 1) you can have constructor for abstract class, but not in trait >> 2) Abstract classes are fully interoperable with Java. You can call them >> from Java code without any wrappers. Traits are fully interoperable only if >> they do not contain any implementation code for scala 2.11. >> 3) you can do multiple inheritance or mixin composition with trait. >> >> In the TableEnvironment case, >> 1) I don't see a need for mixin, and class hierarchy seems fit better here >> by design. >> 2) to better interoperate with Java from scala 2.11, it's better to use >> abstract class. (But AFAIK, scala 2.12 and java 8 would be compatible, >> though) >> 3) you might pay a bit performance overhead with trait (compiled to >> interface) compared to abstract class, but it's not a big deal here. >> >> But in other cases, trait might be a better one if it might be reused and >> mixined in multiple, unrelated classes. >> >> So another option would be to refactor TableEnvironment to clean up or move >> the 'private[flink]' or 'protected' stuff to the actual implementor >> (e.g. 'InternalTableEnvironment') as >> you would do for your trait approach for TableEnvironment. I think this >> option might help with backward compatibility as well. Thanks. >> >> Shuyi >> >> On Fri, Mar 2, 2018 at 10:25 AM, Rong Rong <[hidden email]> wrote: >> >>> Hi Timo, >>> >>> Thanks for looking into this Timo. It's becoming increasingly messy for >> my >>> trying to locate the correct functions in IDE :-/ >>> >>> This is probably due to the fact that Scala and Java access modifiers / >>> qualifiers are subtly and fundamentally different. Using Trait might be >> the >>> best solution here. Another way I can think of is to move the all >>> TableEnvironment classes to Java side, but that would probably introduce >> a >>> lot of issue we need to resolve on the Scala side though. "protected" is >>> less restrictive in Java but there's really no equivalent of package >>> private modifier on Java. >>> >>> I was wondering is there any better way to provide backward-compatible >>> support though. I played around with it and seems like every "protected" >>> field will create a private Java member and a public getter, should we >> add >>> them all and annotate with "@Deprecated" ? >>> -- >>> Rong >>> >>> On Thu, Mar 1, 2018 at 10:58 AM, Timo Walther <[hidden email]> >> wrote: >>>> Hi everyone, >>>> >>>> I'm currently thinking about how to implement FLINK-8606. The reason >>>> behind it is that Java users are able to see all variables and methods >>> that >>>> are declared 'private[flink]' or even 'protected' in Scala. Classes >> such >>> as >>>> TableEnvironment look very messy from the outside in Java. Since we >>> cannot >>>> change the visibility of Scala protected members, I was thinking about >> a >>>> bigger change to solve this issue once and for all. My idea is to >> convert >>>> all TableEnvironment classes and maybe the Table class into traits. The >>>> actual implementation would end up in some internal classes such as >>>> "InternalTableEnvironment" that implement the public traits. The goal >>> would >>>> be to stay source code compatible. >>>> >>>> What do you think? >>>> >>>> Regards, >>>> Timo >>>> >>>> >> >> >> -- >> "So you have to trust that the dots will somehow connect in your future." >> |
Free forum by Nabble | Edit this page |