> >Similarly, I'd suggest to clean up ValueExprList typedefs, > >we have different definitions in different places: > > > >parser/SelectListFactory.h:typedef std::list<ValueExprPtr> ValueExprList; > >query/FuncExpr.h:typedef std::list<ValueExprPtr> ValueExprList; > >query/Predicate.h: typedef std::list<boost::shared_ptr<ValueExpr> > > >ValueExprList; > >query/ValueExpr.h:typedef std::list<ValueExprPtr> ValueExprList; > This is particularly ugly. But, users of ValueExprList do not > necessarily care about ValueExpr, and I would like to break the > transitive dependency as much as I can. If we put the definition in > ValueExpr.h, then users of SelectListFactory.h, FuncExpr.h, etc. > must include ValueExpr.h. > >>Suggestion: let's move SelectStmtList from > >>qana/QueryPlugin.h to query/SelectStmt. > Again, this means that all users of QueryPlugin must use > SelectStmt.h and are sensitive to structural changes there. Most of > the time, the analysis framework only cares that it has an opaque > set of SelectStmt objects. It doesn't need to be recompiled if > SelectStmt changes. Under DRY ("Don't Repeat Yourself"), you can have a ValueExprInterface.h that contains just: class ValueExpr; typedef boost::shared_ptr<ValueExpr> ValueExprPtr; typedef std::list<ValueExprPtr> ValueExprList; and include that in ValueExpr.h. I would not group all such interfaces, particularly if they're unrelated, into a single header. -- Kian-Tat Lim, LSST Data Management, [log in to unmask] ######################################################################## Use REPLY-ALL to reply to list To unsubscribe from the QSERV-L list, click the following link: https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=QSERV-L&A=1