stevedlawrence commented on code in PR #1341:
URL: https://github.com/apache/daffodil/pull/1341#discussion_r1803005632
##########
daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/SpecifiedLengthUnparsers.scala:
##########
@@ -87,122 +86,31 @@ final class SpecifiedLengthExplicitImplicitUnparser(
// but we don't know if the length units characters or bits/bytes.
lengthUnits match {
case LengthUnits.Bits | LengthUnits.Bytes =>
- unparseVarWidthCharactersInBits(state)
- case LengthUnits.Characters =>
- unparseVarWidthCharactersInCharacters(state)
+ eUnparser.unparse1(state)
+ case LengthUnits.Characters => {
+ //
+ // variable-width encodings and lengthUnits characters, and
lengthKind explicit
+ // is not supported (currently) for complex types
+ //
+ state.schemaDefinitionUnless(
+ erd.isSimpleType,
+ "Variable width character encoding '%s', dfdl:lengthKind '%s'
and dfdl:lengthUnits '%s' are not supported for complex types.",
+ getCharset(state).name,
+ lengthKind.toString,
+ lengthUnits.toString
+ )
+
+ Assert.invariant(erd.isSimpleType)
+
+ // truncation is done by StringMaybeTruncateCharactersUnparser
+ eUnparser.unparse1(state, erd)
+ }
Review Comment:
It seems `SpecifiedLengthExplicitImplicitUnparser` doesn't really do
anything anymore. I wonder if it can just be greatly simplified? The binary and
fixed width text cases call unparseBits which just call
`eUnparser.unpase1(...)` (it evaluates target length but I'm not sure if that's
needed anymore?). And now the non-fixed width text case also calls
`eUnparser.unparse1(...)`, just with an SDE check for complex types.
Feels like all of the real work is now moved out of this unparser and into
others. Can this just be simplified to
```scala
if (text representation AND lengthUnits characters AND non fixed width AND
isComplex) {
SDE(...)
} else {
// the child unparser inspects target length and handles truncation and
padding/fill
// nothing needs to be done in this unparser
eUnparser.unparse1(...)
}
```
And literally everything else about targetLength and unparseBits goes away?
--
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]
