================
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:
> 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 :)
No:) The bug is me forgetting to publish my draft comment and then after doing
so, noticing you had already replied.
http://llvm-reviews.chandlerc.com/D273
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
