 |
 |
Jacek, It's not as simple as it appears, unless you don't care about build dependencies. > 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. (Personally, I'm pretty sensitive about having small edits affect large tree of code.) I would be in favor of putting those sorts of typedefs (e.g., typedef std::list<XXX> XXXList;) into a query/containers.h header, so they can all be in one place and avoid introducing dependencies. It has the disadvantage of making a lot more code depend on STL container templates, but I think that's a relatively benign problem because those change rarely. -Daniel ######################################################################## 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