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

Yonik Seeley commented on SOLR-6315:
------------------------------------

If one is unsure of something, questions will go further than bold assertions, 
and avoid putting unnecessary stakes in the ground...  e.g.
"It seems like SimpleOrderedMap doesn't do anything different than NamedList.  
Can it be removed?"
Assertions like "Its javadocs are completely unrelated to the class itself." 
leave me totally at a loss of where to begin... it's like saying "it's all just 
wrong".  Excuse us, we tried to make the javadoc as clear as possible.

So, to the technical issues explaining why the things are the way they are:
1) Solr started with XML only, and NamedList was a container to hold data for 
XML serialization.  NamedList appeared all over in the code.
2) When we added JSON support, we realized that for some data, one would want 
it serialized as a map, and for other data, one would want it serialized as 
something that would preserve order (unfortunately many/most clients that 
consume JSON do not preserve order of "objects").
3) We added a subclass of NamedList, called "SimpleOrderedMap" to make this 
semantic distinction (think of it like a marker interface...).
4) The "JSONResponseWriter hack" you refer to is the entire point of the 
class... to make the semantic distinction to serializers.

So changing all uses of SimpleOrderedMap to NamedList would lose all that 
semantic information (and break back compat for anything other than XML 
responses).

Some of the javadoc snippets are being taken out of context.  I'll quote the 
full javadoc for both NamedList and SimpleOrderedMap for people's reference:

NamedList:
{code}
/**
 * A simple container class for modeling an ordered list of name/value pairs.
 *
 * <p>
 * Unlike Maps:
 * </p>
 * <ul>
 *  <li>Names may be repeated</li>
 *  <li>Order of elements is maintained</li>
 *  <li>Elements may be accessed by numeric index</li>
 *  <li>Names and Values can both be null</li>
 * </ul>
 *
 * <p>
 * A NamedList provides fast access by element number, but not by name.
 * </p>
 * <p>
 * When a NamedList is serialized, order is considered more important than 
access
 * by key, so ResponseWriters that output to a format such as JSON will normally
 * choose a data structure that allows order to be easily preserved in various
 * clients (i.e. not a straight map).
 * If access by key is more important for serialization, see {@link 
SimpleOrderedMap},
 * or simply use a regular {@link Map}
 * </p>
 *
 */
{code}

SimpleOrderedMap:
{code}
/** <code>SimpleOrderedMap</code> is a {@link NamedList} where access by key is 
more
 * important than maintaining order when it comes to representing the
 * held data in other forms, as ResponseWriters normally do.
 * It's normally not a good idea to repeat keys or use null keys, but this
 * is not enforced.  If key uniqueness enforcement is desired, use a regular 
{@link Map}.
 * <p>
 * For example, a JSON response writer may choose to write a SimpleOrderedMap
 * as {"foo":10,"bar":20} and may choose to write a NamedList as
 * ["foo",10,"bar",20].  An XML response writer may choose to render both
 * the same way.
 * </p>
 * <p>
 * This class does not provide efficient lookup by key, it's main purpose is
 * to hold data to be serialized.  It aims to minimize overhead and to be
 * efficient at adding new elements.
 * </p>
 */
{code}

Are there changes to the javadoc we can make that would make it more clear?

> Remove SimpleOrderedMap
> -----------------------
>
>                 Key: SOLR-6315
>                 URL: https://issues.apache.org/jira/browse/SOLR-6315
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>         Attachments: SOLR-6315.patch
>
>
> As I described on SOLR-912, SimpleOrderedMap is redundant and generally 
> useless class, with confusing jdocs. We should remove it. I'll attach a patch 
> shortly.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to