I think the actual implementation is another argument against allowing private methods. I'm not super familiar with it but we actually generate code for calling the annotated methods and that code has to adhere to Java restrictions and cannot call private methods. I think calling private methods is only possible using reflection, which would be a lot slower.
I could be completely wrong, this is just off the top of my hat. On Tue, 17 Jan 2017 at 20:47 Lukasz Cwik <lc...@google.com.invalid> wrote: > 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 > > > > > >
