Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement opt/get methods for accumulated values #551

Open
johnjaylward opened this issue Jul 27, 2020 · 10 comments
Open

Implement opt/get methods for accumulated values #551

johnjaylward opened this issue Jul 27, 2020 · 10 comments

Comments

@johnjaylward
Copy link
Contributor

johnjaylward commented Jul 27, 2020

Currently we have an accumulate method on JSONObject, however, we don't have an easy way to get back out a consistent value (i.e. JSONArray).

One area where this is most painful is in XML to JSON conversion where some XML documents may contain a single element, while other documents may contain many.

Example 1:

<Student>
  <name>jack</name>
  <subjects>
    <class>english</class><grade>98</grade>
  </subjects>
  <subjects>
    <class>english</class><grade>98</grade>
  </subjects>
</Student>

Example 2:

<Student>
  <name>jack</name>
  <subjects>
    <class>english</class><grade>98</grade>
  </subjects>
</Student>

This currently leads to very messy code that has to check the type of value parsed:

void someFunction() {
  JSONObject jo = XML.toJSONObject(someXmlSource);
  JSONObject student = jo.getJSONObject("Student");
  if ( student.optJSONArray("subjects") ==null ) {
      processSubject(student.optJSONObject("subjects"));
  } else {
      processSubjects(student.optJSONArray("subjects"));
  }
}

void processSubjects(JSONArray subjects) {
  if (subjects == null || subjects.length() == 0) return;
  for(int i=0; i<subjects.length; i++) {
    processSubject(subjects.getJSONObject(i));
  }
}

void processSubject(JSONObject subject) {
  // some implementation
}

It would be nice if there was a convenience function in the JSONObject to get values that are accumulated:

// In JSONObject

/**
* Always returns a JSONArray, If the key doesn't exist or the value is `null`, the array will be empty.
* If the key holds a non-array/non-collection value, then it is placed in a JSONArray
* before returning.
* If the value is a JSONArray, then a copy is returned.
* If the value is directly convertible to a JSONArray (i.e. a JAVA Array or Collection), then it is
* converted to a JSONArray.
* @param key The key in the JSONObject to fetch
* @returns a JSONArray regardless of the value of the key.
*/
public JSONArray optAccumulated(String key) {
  Object o = opt(key);
  if (JSONObject.NULL.equals(o)) {
    return new JSONArray();
  }
  if (o instanceof JSONArray) {
    return new JSONArray(((JSONArray)o).myArrayList); // maybe create a copy constructor and/or extend putAll
  }
  if (o instanceof Collection || o.getClass().isArray()) {
    return new JSONArray(o);
  }
  JSONArray ret = new JSONArray();
  ret.put(o);
  return ret;
}


/**
* Always returns a JSONArray, If the key value is `null`, the array will be empty.
* If the key holds a non-array/non-collection value, then it is placed in a JSONArray
* before returning.
* If the value is a JSONArray, then a copy is returned.
* If the value is directly convertible to a JSONArray (i.e. a JAVA Array or Collection), then it is
* converted to a JSONArray.
* @param key The key in the JSONObject to fetch
* @returns a JSONArray regardless of the value of the key.
* @throws JSONException Thrown when the key does not exist
*/
public JSONArray getAccumulated(String key) throws JSONException {
  Object o = get(key);
  if (JSONObject.NULL.equals(o)) {
    return new JSONArray();
  }
  if (o instanceof JSONArray) {
    return new JSONArray(((JSONArray)o).myArrayList); // maybe create a copy constructor and/or extend putAll
  }
  if (o instanceof Collection || o.getClass().isArray()) {
    return new JSONArray(o);
  }
  JSONArray ret = new JSONArray();
  ret.put(o);
  return ret;
}

See also #550

@stleary
Copy link
Owner

stleary commented Jul 28, 2020

Agreed that the XML transformation is a tradeoff, sometimes a bad one. But would prefer to see something like an XMLParserConfiguration with() option that lets the user customize parsing. Using opt() and get() methods to change the output from the internal representation doesn't seem like the best solution, at least to me.

@LibralCoder
Copy link

Can we add some customize rule In the process of converting xml to json?

@stleary
Copy link
Owner

stleary commented Jul 30, 2020

No objection to allowing developers to customize parsing. We already have XMLParserConfiguration, should try to use that first.

@kovax
Copy link

kovax commented Sep 30, 2020

My project is also heavily relies on the XML to JSON functionality of json.org, and it would be perfect if the XML.toJSONObject(someXmlSource); could be configured with a list of fields names (perhaps list of xpathes), that are enforced to produce JSONArray. All of our xml data has an XSD, which can be be used to determine the multiplicity of the element (maxOccurs > 1), so ultimately this transformation could be based on the XSD itself.

@johnjaylward
Copy link
Contributor Author

We don't currently have any xpath support in the project, and I don't foresee us ever supporting XSD. However we do have a JSONPath implementation that could be used to implement something similar to your idea, but on the result side instead of the source side.

@kovax
Copy link

kovax commented Oct 1, 2020

@johnjaylward I guess your solution patches the JSONObject after the XML to JSON conversion. This is a good workaround, I should have thought about it :) , but I still would prefer if the framework could support.

BTW, I am sorry that I mentioned XSD, I know it would be way too complex to use here ...

@914-Chu
Copy link

914-Chu commented Oct 28, 2021

Hi, is this issue still open? May I take it? Thank you!

@stleary
Copy link
Owner

stleary commented Oct 28, 2021

Hi, @914-Chu, assigning this issue to you.

@914-Chu
Copy link

914-Chu commented Oct 28, 2021

Thank you!

@stleary
Copy link
Owner

stleary commented Sep 30, 2023

It seems to me that the XmlParserConfiguration.forceList property added in #646 addresses this issue. If so, I will close it. Please let me know if you think work still needs to be done.
@johnjaylward @LibralCoder @kovax @914-Chu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants