: Attached refresh.

I've been too busy the past two days to play with this -- or even read
it carefully, but here's some impressions as i skim it...


1) This looks like an idiom in the making ... it might be worth
while to go ahead and refactor into a static utility now
(ie: "SolrParams p = SolrParams.wrapDefaults(req,defaults)") ...

+      SolrParams p = req.getParams();
+      if (defaults != null) {
+        p = new DefaultSolrParams(p,defaults);
+        // set params so they will be visible to other components such as the 
response writer
+        req.setParams(p);
+      }


2) ...

+      // TODO: We have some constants in SolrQueryRequestBase, and some in 
CommonParams...

...i vote for pulling them out of SolrQueryRequestBase ... i would go so
far as to recommend deprecating the named accessors like getQueryString,
getLimit, and getStart -- the only params that should be treated special
are "qt" and "wt".  As for where they should live, ... I'm thinking
CommonParams is headed the way of the Dodo, SolrParams seems like the
right place to me.


3) in this method, would it be better to let getOriginalParams() return
null? .. as is information (the lack of params when the request was
constructed) can be lost if setParams is called more then once...

+  public void setParams(SolrParams params) {
+    if (this.origParams==null) this.origParams=params;
+    this.params = params;
+  }






-Hoss

Reply via email to