 |
 |
Daniel at al, You keep asking about feedback re existing code and how to make it better. While looking around, I realized I am uneasy about how open some classes are, QueryContext perhaps is the leader in that respect. There is a bunch of very loosely coupled things in there that are all public, pointers to QueryContext are lying around "everywhere", and we keep poking at it from so many places I completely lost track who is doing what when and why, who is reading only, and who is actually modifying what. So, I tried to mess around with it a little, please note that this is experimental, more like to explore things. Have a look at my branch u/jbecla/tightningQueryContext. I isolated _restrictors, pushed it to a dedicated class, made it private and added modifiers/accessors. I started doing it for others. While doing it I notices some code belongs near QueryContext instead of QuerySession. What I have now gives me a much better picture who is touching _resolvers. I want to try doing something similar to other pieces wherever it makes sense (ScanTables, Restrictors, maybe Aliases, maybe QueryMapping, etc). I did not want to be too intrusive so I currently did the isolation through (multiple) inheritance, but I'd be completely open to other options. I am thinking that maybe we should go one step further and take apart QueryContext, get rid of it, and replace it with a few specialized QueryContext-like objects, owned by QuerySession. The reason why: I discovered that_restrictors are really touched only by QservRestrictorPlugin...., so perhaps there is a pattern here and certain plugins touch certain parts only. Please have a look and let me know what your reaction is (the sooner the better, if I find some time here and there I will be playing with that code more). Along these lines, I think I'd be in favor of tightening other classes and introducing modifiers/accessors instead of keeping class members public (except really trivial classes like css/StripingParams). Brickbats welcome, but I am hiding far far away, so they won't reach me :) Thanks, Jacek ######################################################################## 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