[ 
https://issues.apache.org/jira/browse/SOLR-388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12537505
 ] 

Hoss Man commented on SOLR-388:
-------------------------------

Sorry, i'm a little behind on some things, several comments...

> Hoss. I can certainly copy/convert the results into a NamedList composed only 
> of the primitives. But it's an extra copy that's bad for the performance

you don't have to copy anything, you just have to make sure that whatever 
complex object you want to add to the response implements one of the allowed 
interfaces (Map or List probably being the easiest) so the ResponseWriter's can 
access them.

As i said before: please start a thread on the solr-user list describing what 
it is you are doing in your RequestHandler, and the community might have lots 
suggestions for achieving your goals in a performant way that can work with any 
response writer (and won't require you to hack any internals or wait on any API 
changes)

> BTW, the documentation needs to be updated to reflect that Document is 
> also a supported primitive.

ehhhh...  it's a question of perspective.  the documentation is correct in that 
response writers have never been required to support Document as a primitive 
(just DocList).  i haven't looked closely, but it wouldn't surprise me if the 
recursive nature of the writers that come with Solr can handle it okay, but 
that doesn't mean the docs for the interface are wrong ... it just means that 
example impl's do more then they have to.

> Are there any reasons why it would be bad for JSONWriter to be public, 
> especially 
> considering XMLWriter has been public for a while?

only that making a class public is essentially a one way operation; once a 
class is public it can't easily be made unpublic, nor can any of it's method 
signatures be changed .. which can be very limiting in making future 
improvements.  adding a package protected class with a "dirty" API can be done 
easily, because we don't have to maintain it if we decide we don't like it -- 
we control all the clients and can change it all at once.  So deciding to make 
a class public requires careful consideration as to wether or not we think the 
API of that class is "dirty" and if Solr is willing to stand by it for the 
foreseeable future.

> Refactor ResponseWriters and Friends.
> -------------------------------------
>
>                 Key: SOLR-388
>                 URL: https://issues.apache.org/jira/browse/SOLR-388
>             Project: Solr
>          Issue Type: Improvement
>          Components: search
>    Affects Versions: 1.2
>            Reporter: Luke Lu
>
> When developing custom request handlers, it's often necessary to create 
> corresponding response writers that extends existing ones. In our case, we 
> want to augment the result list (more attributes other than numFound, 
> maxScore, on the fly per doc attributes that are not fields etc.) , only to 
> find JSONWriter and friends are private to the package. We could copy the 
> whole thing and modify it, but it wouldn't take advantage of recent fixes 
> like Yonik's FastWriter changes without tedious manual intervention. I hope 
> that we can can *at least* extends it and overrides writeVal() to add a new 
> result type to call writeMyType. 
> Ideally the ResponseWriter hierarchy could be rewritten to take advantage of 
> a double dispatching trick to get rid of the ugly if something is instance of 
> someclass else ... list, as it clearly doesn't scale well with number of 
> types (_n_) and depth (_d_) of the writer hierarchy, as the complexity would 
> be O(_nd_), which worse than the O(1) double dispatching mechanism. Some 
> pseudo code here:
> {code:title=SomeResponseWriter.java}
> // a list of overloaded write method
> public void write(SomeType t) {
>   // implementation
> }
> {code}
> {code:title=ResponseWritable.java}
> // an interface for objects that support the scheme
> public interface ResponseWritable {
>   public abstract void write(ResponseWriter writer);
> }
> {code}
> {code:title=SomeType.java}
> // Sometype needs to implement the ResponseWritable interface
> // to facilitate double dispatching
> public void write(ResponseWriter writer) {
>   writer.write(this);
> }
> {code}
> So when adding a new MyType and MySomeResponseWriter, we only need to add 
> these two files without having to muck with the writeVal if-then-else list. 
> Note, you still need to use the if else list for builtin types and any types 
> that you can't modify in the write(Object) method. 
> {code:title=MyType.java}
> // implements the ResponseWritable interface
> public write(ResponseWriter writer) {
>   writer.write(this);
> }
> {code}
> {code:title=MySomeResponseWriter.java}
> //  only need to implement this method
> public void write(MyType t) {
>   // implementation
> }
> {code}

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to