I'm in favor of the suggestion. Do you guys plan to do a single sweep or document the pattern somewhere and apply it only for new and refactored code?
On Wed, Jun 28, 2017 at 12:19 AM, Yan Xu <xuj...@apple.com> wrote: > This sounds reasonable to me. Do others have comments? > > --- > @xujyan <https://twitter.com/xujyan> > > On Fri, Jun 23, 2017 at 4:23 PM, James Peach <jor...@gmail.com> wrote: > > > Hi all, > > > > There is a common Mesos pattern where a subsystem is implemented by a > > facade class that forwards calls to an internal Process class, eg. > Fetcher > > and FetcherProcess, or zookeeper::Group and zookeeper::GroupProcess. > Since > > the Process is an internal implementation detail, I'd like to propose > that > > we adopt a general policy that it should not be exposed in the primary > > header file. This has the following benefits: > > > > - reduces the number of symbols exposed to clients including the primary > > header file > > - reduces the number of header files needed in the primary header file > > - reduces the number of rebuilt dependencies when the process > > implementation changes > > > > Although each individual case of this practice may not improve build > > times, I think it is likely that over time, consistent application of > this > > will help. > > > > In many cases, when FooProcess is only used by Foo, both the declaration > > and definitions of Foo can be inlined into "foo.cpp", which is already > our > > common practice. If the implementation of the Process class is needed > > outside the facade (eg. for testing), the pattern I would propose is: > > > > foo.hpp - Primary API for Foo, forward declares FooProcess > > foo_process.hpp - Declarations for FooProcess > > foo_process.cpp - Definitions of FooProcess > > > > The "checks/checker.hpp" interface almost follows this pattern, but gives > > up the build benefits by including "checker_process.hpp" in > "checker.hpp". > > This should be simple to fix however. > > > > thanks, > > James >
