[GitHub] stevedlawrence commented on a change in pull request #158: Daffodil 1080 sequences and separators - preliminary review

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

stevedlawrence commented on a change in pull request #158: Daffodil 1080 sequences and separators - preliminary review
URL: https://github.com/apache/incubator-daffodil/pull/158#discussion_r242148901
 
 

 ##########
 File path: daffodil-core/src/main/scala/org/apache/daffodil/dsom/SequenceGroup.scala
 ##########
 @@ -103,39 +101,41 @@ abstract class SequenceGroupTermBase(
 
   protected def hiddenGroupRefOption: PropertyLookupResult
 
-  /**
-   * True if this sequence group has syntactic features itself or
-   * within itself.
-   */
-  final lazy val hasStaticallyRequiredOccurrencesInDataRepresentation = {
-    // true if there are syntactic features
-    hasInitiator || hasTerminator ||
-      // or if any child of the sequence has statically required instances.
-      groupMembers.exists { _.hasStaticallyRequiredOccurrencesInDataRepresentation }
-  }
+  final override lazy val hasKnownRequiredSyntax = LV('hasKnownRequiredSyntax) {
+    if (hasFraming) true
+    else {
+      val representedMembers = groupMembers.filter { _.isRepresented }
 
-  final override def hasKnownRequiredSyntax = LV('hasKnownRequiredSyntax) {
-    lazy val memberHasRequiredSyntax = groupMembers.exists(_.hasKnownRequiredSyntax)
-    lazy val prefixOrPostfixAndStaticallyRequiredInstance =
-      groupMembers.filter { _.isRepresented }.exists { _.hasStaticallyRequiredOccurrencesInDataRepresentation } &&
-        (hasPrefixSep || hasPostfixSep)
-    lazy val infixAnd2OrMoreStaticallyRequiredInstances =
-      groupMembers.filter { m => m.isRepresented && m.hasStaticallyRequiredOccurrencesInDataRepresentation }.length > 1 && hasInfixSep
-    lazy val sepAndArryaWith2OrMoreStaticallyRequiredInstances =
-      groupMembers.filter { m =>
-        m.isRepresented && m.hasStaticallyRequiredOccurrencesInDataRepresentation && (m match {
-          case e: ElementBase => e.minOccurs > 1
-          case _ => false
-        })
-      }.length > 0 && hasSeparator
-    val res =
-      hasInitiator ||
-        hasTerminator ||
+      lazy val memberHasRequiredSyntax = representedMembers.exists { member =>
+        val res = member.hasKnownRequiredSyntax
+        res
+      }
+      lazy val prefixOrPostfixAndStaticallyRequiredInstance =
+        representedMembers.exists { member =>
+          val res = member.hasStaticallyRequiredOccurrencesInDataRepresentation
+          res
+        } &&
+          (hasPrefixSep || hasPostfixSep)
+      lazy val infixAnd2OrMoreStaticallyRequiredInstances =
 
 Review comment:
   Maybe change 2 to Two? At first I read this as 20 or more and was very confused.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services