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 <suez1...@gmail.com> 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 <walter...@gmail.com> 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 <twal...@apache.org>
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."
