tuxji commented on code in PR #1122:
URL: https://github.com/apache/daffodil/pull/1122#discussion_r1408432865
##########
daffodil-japi/src/main/scala/org/apache/daffodil/japi/Daffodil.scala:
##########
@@ -141,7 +141,6 @@ class Compiler private[japi] (private var sCompiler:
SCompiler) {
): ProcessorFactory = {
val pf = sCompiler.compileFile(schemaFile, Option(rootName),
Option(rootNamespace))
- pf.isError
new ProcessorFactory(pf)
Review Comment:
Lines 143 - 144 look strange at first glance. We get a pf which is a
ProcessorFactory and wrap another ProcessorFactory around it. I do realize
that pf is actually a core Daffodil ProcessorFactory class and the
ProcessorFactory on line 144 is actually a Java API ProcessorFactory class.
Nevertheless, I wish we could avoid using these Java and Scala API wrapper
classes if possible.
##########
daffodil-sapi/src/test/scala/org/apache/daffodil/example/TestScalaAPI.scala:
##########
@@ -1325,4 +1325,40 @@ class TestScalaAPI {
Files.delete(blobDir)
}
}
+
+ /**
+ * Verify that ProcessorFactory.withDistinguishedRootNode selects the right
node
+ */
+ @Test
+ def testScalaAPIWithDistinguishedRootNode(): Unit = {
+ val c = Daffodil.compiler()
+
+ // e3 is defined first in mySchema3.dfdl.xsd, so if
withDistinguishedRootNode is ignored,
+ // this should give a different result
+ val schemaFile = getResource("/test/sapi/mySchema3.dfdl.xsd")
+ val pf = c
+ .compileFile(schemaFile)
+ .withDistinguishedRootNode("e4", null)
+ val dp1 = pf.onPath("/")
Review Comment:
Aren't we supposed to call pf.isError before we call pf.onPath("/")?
##########
daffodil-japi/src/test/java/org/apache/daffodil/example/TestJavaAPI.java:
##########
@@ -1346,4 +1346,38 @@ public void testJavaAPIBlob1() throws IOException,
ClassNotFoundException, Inval
Files.delete(blobDir);
}
}
+
+ /**
+ * Verify that ProcessorFactory.withDistinguishedRootNode selects the
right node
+ */
+ @Test
+ public void testJavaAPIWithDistinguishedRootNode() throws IOException,
ClassNotFoundException {
+ org.apache.daffodil.japi.Compiler c = Daffodil.compiler();
+
+ // e3 is defined first in mySchema3.dfdl.xsd, so if
withDistinguishedRootNode is ignored,
+ // this should give a different result
+ java.io.File schemaFile = getResource("/test/japi/mySchema3.dfdl.xsd");
+ ProcessorFactory pf = c.compileFile(schemaFile);
+ pf = pf.withDistinguishedRootNode("e4", null);
+ DataProcessor dp = pf.onPath("/");
Review Comment:
Aren't we supposed to call pf.isError before we call pf.onPath("/")?
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/compiler/Compiler.scala:
##########
@@ -92,13 +89,10 @@ final class ProcessorFactory private (
validateDFDLSchemas,
checkAllTopLevel,
tunables,
- Some(sset),
Review Comment:
I think requiring that a user call isError before calling some
ProcessorFactory methods is potentially confusing. I would like to know more
about the use case(s) where someone gets a ProcessorFactory but abstains from
calling isError on purpose. What is the justification or motivation for such
use case(s)? Is the justification that a user might want to call various
withXXX methods on the ProcessorFactory to fine tune the ProcessorFactory's
parameters and configuration before actually using the ProcessorFactory to get
a Processor?
If that is the justification and the motivation for it is compelling enough
(avoid doing a considerable amount of work more than once), then this PR's
changes will let the ProcessorFactory initialize only part of itself and let
the user finish the initialization by calling isError only after completing all
of the withXXX calls. I would say that maybe we should find a better API
design, but I know it's not easy to design an API that can be used by both Java
and Scala programmers. I will not argue with this ProcessorFactory withXXX /
isError API design.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
