stevedlawrence commented on a change in pull request #75: Daffodil 1738 zoned
decimal
URL: https://github.com/apache/incubator-daffodil/pull/75#discussion_r195399373
##########
File path:
daffodil-lib/src/main/scala/org/apache/daffodil/util/DecimalUtils.scala
##########
@@ -335,4 +335,142 @@ object DecimalUtils {
outArray
}
+ def convertFromAsciiStandard(digit: Int): (Int, Boolean) = {
+ if ((digit >= 48) && (digit <= 57)) // positive 0-9
+ return (digit - 48, false)
+ else if ((digit >= 112) && (digit <= 121)) // negative 0-9
+ return (digit - 112, true)
+ else
+ throw new NumberFormatException("Invalid zoned digit: " + digit)
+ }
Review comment:
Avoid return statemens. Scala implements them by throwing/catching
exceptions which could potentially have performance implications.
Also, I think this function deserves a java doc. I'm not sure what its
doing. It looks like the first part converts the ASCII digits from 0-9 to their
numeric values and false. But then the second converts ascii values 'p' to 'y'
to 0-9 decimal? I assume this is a zoned specific thing. Maybe rename to make
it clear this is doing some zoned conversion? Also not sure what the boolean
represents.
Also, looks like all uses of this pass in a char, should the digit parameter
be a Char as well? You should still be able to do the same numeric comparisons,
but could also do char comparisions, e.g. digit >= '0' or digit <= '9'. That
might be more readable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
