mbeckerle commented on code in PR #1454:
URL: https://github.com/apache/daffodil/pull/1454#discussion_r1989986724
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/DFDLDefineVariable.scala:
##########
@@ -177,7 +176,7 @@ final class DFDLSetVariable(node: Node, decl:
AnnotatedSchemaComponent)
extends VariableReference(node, decl)
with DFDLSetVariableRuntime1Mixin {
private lazy val attrValue = getAttributeOption("value")
- private lazy val <dfdl:setVariable>{eltChildren @ _*}</dfdl:setVariable> =
node
+ private lazy val eltChildren = node.nonEmptyChildren
Review Comment:
This is ok, but it is not equivalent as it does not verify that the node is
a setVariable element.
This would be better:
```
private lazy val Elem("dfdl", "setVariable", _, _, eltChildren) = node
```
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ChoiceGroup.scala:
##########
@@ -74,12 +75,12 @@ trait ChoiceDefMixin extends AnnotatedSchemaComponent with
GroupDefLike {
new DFDLChoice(newDFDLAnnotationXML("choice"), this)
lazy val xmlChildren = xml match {
- case <choice>{c @ _*}</choice> => c
- case <group>{_*}</group> => {
+ case c if c.label == "choice" => c.nonEmptyChildren
Review Comment:
Hmmm. I expected this to become pattern matching on the Elem class:
```
case Elem(_, "choice", _, _, c) => c
case ELem(_,"group", _, _, g) => .....
```
The args to Elem are (prefix, label, attributes, namespaceBindings,
children).
Pattern matching like this is semantically identical to the XML
destructuring.
I.e., 'c' is bound to a Seq[Node] just as in the XML destructuring.
Your solution has changed the meaning of 'c' to be the parent of those
children.
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ChoiceGroup.scala:
##########
@@ -74,12 +75,12 @@ trait ChoiceDefMixin extends AnnotatedSchemaComponent with
GroupDefLike {
new DFDLChoice(newDFDLAnnotationXML("choice"), this)
lazy val xmlChildren = xml match {
- case <choice>{c @ _*}</choice> => c
- case <group>{_*}</group> => {
+ case c if c.label == "choice" => c.nonEmptyChildren
+ case g if g.label == "group" => {
val ch = this match {
case cgd: GlobalChoiceGroupDef => cgd.xml \ "choice"
}
- val <choice>{c @ _*}</choice> = ch(0)
+ val c = ch(0).nonEmptyChildren
Review Comment:
This would be:
```
val Elem(_, "choice", _, _, c) => c
```
I just think this is much cleaner and more compact.
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/DFDLDefineFormat.scala:
##########
@@ -35,8 +35,10 @@ final class DFDLDefineFormat(node: Node, sd: SchemaDocument)
{
lazy val formatAnnotation = LV(Symbol("formatAnnotation")) {
- XMLUtils.removeComments(Utility.trim(node)) match {
- case <defineFormat>{f @ <format>{_*}</format>}</defineFormat> =>
+ val cleanedNode = XMLUtils.removeComments(Utility.trim(node))
Review Comment:
This could be:
```
XMLUtils.removeComments(Utility.trim(node)) match {
case Elem(_, "defineFormat", _, _, Seq(f @ Elem(_, "format", _, _, _)))
=> DFDLFormat(f, sd)
case _ => ...
```
Even in this slightly more complex case, it's easier to do with the pattern
matches and the code is far less cluttered.
--
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]
