tuxji commented on code in PR #917:
URL: https://github.com/apache/daffodil/pull/917#discussion_r1081755476


##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/TypeCalculator.scala:
##########
@@ -176,17 +176,17 @@ class KeysetValueTypeCalculatorUnordered(valueMap: 
HashMap[DataValuePrimitive, D
     if (valueMap.contains(x)) {
       valueMap.get(x) match {
         case Some(a) => (a, Maybe.Nope)
-        case None => (DataValue.NoValue, One(s"Value ${x} not found in 
keyset-value mapping"))
+        case None => (DataValue.NoValue, One(s"Value ${x} not found in 
enumeration dfdlx:repValues"))

Review Comment:
   Line 179 will never be hit because of the `if (valueMap.contains(x))` at 
line 176.  I suggest you get rid of the if statement and its else branch, 
leaving only this match expression as this method's body.  Or else get rid of 
the match expression and replace it with the line `(valueMap.get(x), 
Maybe.Nope)`.



##########
daffodil-test/src/test/resources/org/apache/daffodil/extensions/type_calc/inputTypeCalc.tdml:
##########
@@ -292,16 +292,35 @@
       <xs:restriction base="xs:string">
         <xs:enumeration value="VALUE=0" dfdlx:repValues="0"/>
         <xs:enumeration value="VALUE=1" dfdlx:repValues="1"/>
+        <!--<xs:enumeration value="VALUE=2" dfdlx:repValueRanges="4 5"/>-->
       </xs:restriction>

Review Comment:
   It looks like you added line 295 but commented it out and added 
"enumWithRange" below instead?  I think you should remove this line so there's 
no commented out code.



##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/TypeCalculator.scala:
##########
@@ -163,7 +163,7 @@ class KeysetValueTypeCalculatorOrdered(valueMap: 
HashMap[DataValuePrimitive, Dat
   override def outputTypeCalc(x: DataValuePrimitive, xType: NodeInfo.Kind): 
(DataValuePrimitiveNullable, Maybe[Error]) = {
     unparseMap.get(x) match {
       case Some(v) => (v, Maybe.Nope)
-      case None => (DataValue.NoValue, One(s"Value ${x} not found in 
keyset-value mapping"))
+      case None => (DataValue.NoValue, One(s"Value ${x} not found in 
enumeration dfdlx:repValues"))

Review Comment:
   Is there a reason why the message here at line 166 is not identical to the 
message at line 156?  That is, why does the other message mention only 
"enumeration" while this message mentions both "enumeration" and 
"dfdlx:repValues"?
   
   BTW, I think the order should be reversed: "not found in dfdlx:repValues 
enumerations".



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