[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #416: Throw SDE in the case of a choice with no branches

[GitBox <gi...@apache.org>]

stevedlawrence commented on a change in pull request #416:
URL: https://github.com/apache/incubator-daffodil/pull/416#discussion_r487076908



##########
File path: daffodil-test/src/test/scala/org/apache/daffodil/section15/choice_groups/TestChoice.scala
##########
@@ -123,9 +124,6 @@ class TestChoice {
   @Test def test_direct_dispatch_16(): Unit = { runnerCH.runOneTest("direct_dispatch_16") }
   @Test def test_direct_dispatch_17(): Unit = { runnerCH.runOneTest("direct_dispatch_17") }
 
-  //@Test def test_choice_noBranch() { runnerCH.runOneTest("choice_noBranch") } - Test consumes no data, which causes a TDMLError
-
-

Review comment:
       Why did you delete the choice_noBranch test and replace it with a new one? Was something wrong with choice_noBranch? That name feels more appropriate than optionalChoice00, since this isn't testing optional choices.

##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dsom/ChoiceGroup.scala
##########
@@ -201,6 +202,12 @@ abstract class ChoiceTermBase(
     }
   }.value
 
+  final lazy val noBranchesFound = LV('noBranchesFound) {
+    if(groupMembers.size.equals(0)) {

Review comment:
       The standard in scala is to use ``==`` instead of ``.equals()``. Also note that there should be a space between ``if`` and the open paren.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org