Hi Jesse, can you specifically say which functions on Combine and Count you're thinking of? I believe these transforms are consistent with the "principle of least visibility" -- make nothing more public than it needs to be.
Look at Combine.globally <https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/Combine.java#L124>. It returns a Globally, but that is because Globally has a useful public API surface, adding functions like asSingletonView. I believe similar reasoning applies to Count. However, for cases where the user will not further configure the return value, it makes sense to return the most public type we can. On Fri, Jan 27, 2017 at 6:39 AM, Jesse Anderson <[email protected]> wrote: > One con to making transform classes be private would be that it is a > breaking change. If anyone uses that class directly or extends that class, > we'd be breaking that. > > On Fri, Jan 27, 2017 at 9:37 AM Jesse Anderson <[email protected]> > wrote: > > > Continuing a discussion <https://github.com/apache/beam/pull/1830> Dan, > > Kenn, and I were having here since the bug is closed. They pointed out > > three things: > > > > - Where the private constructor gets placed in the class > > - Where the code samples of how to use the class get placed (in the > > Transform versus in the static method) > > - Are transform classes public or private > > > > I noted that those were inconsistent in the code. When I write a new > > transform, I use one of the already written transforms as the basis. > > > > Looking at Combine and Count: > > > > - The private constructor is at the top of the class > > - The code sample is in the Transform class > > - The transform class is marked as public > > > > I don't have a strong opinion on private constructor and transform being > > marked as private/public. I think we should standardize on placing code > > samples in the static helper methods. That's where people are looking > when > > using these methods. > > > > I think we need to do a general pass to make these consistent after we > > decide on how they should be done. > > > > Thanks, > > > > Jesse > > >
