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