XMLParserConfiguration needs to be opened up.

[stleary]

High priority - at least one pull request is waiting on this.
See comments in #540 and #412
Existing XMLParserConfiguration constructors to be deprecated.
Add property setters.
Add default constructor.

[johnjaylward]

I don't think public setters on the configuration object is a good idea. For instance, if someone wanted to parse 2 or more XML files on different threads they could do something like this:

void main() {
  XMLParserConfiguration myConfig = new XMLParserConfiguration();
  myConfig.option1 = someSetting;
  myConfig.option2 = someOtherSetting;

  ConcurrentList<JSONObject> cl = new ConcurrentList<JSONObject>();

  XmlFileList.stream().parallel().foreach((f) -> cl.add(processFile(f, myConfig)));
  // do something with the completed list of JSONObjects
}

JSONObject processFile(File f, XMLParserConfiguration config) {
  if (f.getName().contains(something)) {
    // this will incorrectly be set the wrong file. It's not thread safe but is not at-a-glance wrong here.
    // however, the "config" option is used by multiple threads
    config.option1 = someSetting3; 
  }
  try (FileReader fr = new FileReader(f)) {
    return XML.toJSON(fr, myConfig);
  }
}

What I would prefer is instead to use a withOption and leave the values as private final.

What this could mean is that we still have a single simple constructor, but building up your config would have more GC overhead.

using the example above, similar code would look list this:

void main() {
  XMLParserConfiguration myConfig = new XMLParserConfiguration();
  myConfig = myConfig.withOption1(someSetting);
  myConfig = myConfig.withOption2(someOtherSetting);

  ConcurrentList<JSONObject> cl = new ConcurrentList<JSONObject>();

  XmlFileList.stream().parallel().foreach((f) -> cl.add(processFile(f, myConfig)));
  // do something with the completed list of JSONObjects
}

JSONObject processFile(File f, XMLParserConfiguration config) {
  if (f.getName().contains(something)) {
    // this will correctly be set for only this file. It's thread safe and is at-a-glance correct.
    config = config.withOption1(someSetting3); 
  }
  try (FileReader fr = new FileReader(f)) {
    return XML.toJSON(fr, myConfig);
  }
}

[stleary]

That could work. What if someone needs to override multiple defaults on a single config?
For example, not executing in parallel, if the XML needs to configure keepStrings, convertNilAttributeToNull, and typeCastClass in a single XMLParserConfiguration instance?

[johnjaylward]

XMLParserConfiguration myConfig = new XMLParserConfiguration()
    .withOption1(setting1)
    .withOption2(setting2)
    .withOption3(setting3)
;

the with* methods would all return the new configuration, so you can chain them like a builder.

[stleary]

Sounds good! Do you want to add the code?