================
Comment at: lib/Tooling/Refactoring.cpp:142
@@ +141,3 @@
+// saveRewrittenFiles() and getResults(), without cluttering Refactoring.h's
+// header with includes that are really only needed by the implementation.
+class RewriteHelper {
----------------
Manuel Klimek wrote:
> Edwin Vane wrote:
> > Edwin Vane wrote:
> > > Manuel Klimek wrote:
> > > > I think not putting something into the header is not a good reason to
> > > > pull out an abstraction. I also generally don't like abstractions
> > > > called "Helper" (I understand that you don't intend this to be an
> > > > abstraction, but my point is that if there's not abstraction, I don't
> > > > think it makes sense to pull one out just to reduce stuff in the
> > > > header).
> > > Alright. I'm still new to this and take what I read in the LLVM coding
> > > standards at face value. I'm referring to the "include as little as
> > > possible". But even from a design standpoint, is it really necessary to
> > > expose the types that are really only needed by the implementation to
> > > every user of Refactoring.h?
> > With the three-method approach this point is moot but I'm still interested
> > in the answer in general.
> Well, to me it depends on how it affects the design. I'm all for pulling out
> abstractions that make sense, especially when they are just implementation
> details.
>
> But just not wanting something in the (private section of a class in the)
> header is for me not enough of an argument on its own.
>
> Obviously all those are judgement calls where opinions and taste can differ,
> so I'm always open to a good argument that might change my mind in a specific
> case :)
Did you not see my answer for some reason? If so, I'll file a phab bug :)
http://llvm-reviews.chandlerc.com/D273
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
