Requiring public is also a form of documentation for implementers and maintainers that this method is meant to be called by someone else.
All our arguments have been around best practices. I think the only technical argument is that if a runner today chooses to execute with a security manager, it may disallow making private methods accessible. From my knowledge, none of the runners currently use a security manager so its a weak point. On Tue, Jan 17, 2017 at 10:07 AM, Stas Levin <stasle...@gmail.com> wrote: > We stumbled across this issue when we were doing some Scala-Java interop > and had an anonymous class that was supposed to have an element processing > method. The Scala compiler had decided to make that method private, and so > it was not visible to Beam. While extracting this class to an upper level > (i.e., non-anonymous) solved the problem, it made me wonder. > > I can see the "overrides-in-spirit" point, but since it's not really an > interface in the regular sense (e.g., no polymorphism) I can also imagine > cases where users may want to encapsulate (i.e. make private) the element > processing logic inside a class. It's already a reflection based mechanism > so the difference between inspecting public only vs. all may not be that > big, and as far as users are concerned both are "magic" :) > > IMHO, the same argument could apply to JUnit, and as a JUnit user, I'm not > sure I can see the benefit of forcing test methods and rules to be public > in the scope of a test class. Some would even say this may not be ideal in > terms of encapsulation. I might be missing something here, so I wanted to > get some more perspective. > > On Tue, Jan 17, 2017 at 6:47 PM Ben Chambers <bchamb...@google.com.invalid > > > wrote: > > > We thought about it when this was added. We decided against it because > > these are overrides-in-spirit. Letting them be private would be > misleading > > because they are called from outside the class and should be thought of > in > > that way. > > > > Also, this seems similar to others in the Java ecosystem: JUnit test > > methods are public as well. > > > > What is the motivation for allowing private methods? > > > > > > On Tue, Jan 17, 2017, 7:33 AM Stas Levin <stasle...@gmail.com> wrote: > > > > Hi, > > > > At the moment only public methods are eligible to be decorated with > > @ProcessElement (DoFnSignatures#findAnnotatedMethod). > > > > Seems that from a technical point of view, it would be quite possible to > > consider private methods as well. In fact, it's one of the benefits of > > moving towards an annotation based discovery as opposed to an interface > > based one. > > > > Do you think being able to decorate non-public methods with > @ProcessElement > > (and similar annotations) is something we'd like to support? > > > > Regards, > > Stas > > >
