stevedlawrence commented on code in PR #926:
URL: https://github.com/apache/daffodil/pull/926#discussion_r1084033198


##########
daffodil-test/src/test/scala/org/apache/daffodil/section05/simple_types/TestSimpleTypesUnparse.scala:
##########
@@ -50,7 +50,9 @@ class TestSimpleTypesUnparse {
   @Test def test_hexBinary_unparse_10(): Unit = { 
runner.runOneTest("hexBinary_unparse_10") }
   @Test def test_hexBinary_unparse_11(): Unit = { 
runner.runOneTest("hexBinary_unparse_11") }
   @Test def test_hexBinary_unparse_12(): Unit = { 
runner.runOneTest("hexBinary_unparse_12") }
+  @Test def test_hexBinary_unparse_13(): Unit = { 
runner.runOneTest("hexBinary_unparse_13") }

Review Comment:
   At the top of this we have this test commented out. That can be removed now.



##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/XSHexBinary.scala:
##########
@@ -67,7 +67,7 @@ trait HexBinaryKind {
       case bi: JBigInt if (bi.bitLength <= 63) => reduce(bi.longValue())
       case bi: JBigInt => bi.toByteArray()
       case bd: JBigDecimal if (try { bd.toBigIntegerExact(); true } catch { 
case e: ArithmeticException => false }) => reduce(bd.toBigIntegerExact())
-      case str: String => reduce(new JBigInt(str))
+      case str: String => reduce(new JBigInt(str, 16))

Review Comment:
   Since the spec says `dfdl:hexBinary behaves identically to the XPath 2.0 
constructor function of the same name...` is there any value in making it so 
the string case is exactly the same as the `XSHexBinary` constructor? We can't 
really call its run function since we need an instance, but it just calls 
`StringToHexBinary.computeValue(...)`. Should we do the same? Note that that 
just calls the above `hexStringToByteArray` function. Maybe we should call that 
instead?
   
   And to make things more confusing, we also have a `Misc.hex2Bytes` function, 
which seems like it has basically the same logic but throws an 
IllegalArgumentException's instead of NumberFormatException's. Maybe we should 
drop the hexStringToByteArray function in favor of Misc.hex2Byte, and change it 
to throw NumberFormatExceptions?



-- 
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]

Reply via email to