[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #253: Support terminators and initiators containing %ES;

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

mbeckerle commented on a change in pull request #253: Support terminators and initiators containing %ES;
URL: https://github.com/apache/incubator-daffodil/pull/253#discussion_r298289984
 
 

 ##########
 File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DelimiterIterator.scala
 ##########
 @@ -17,85 +17,48 @@
 
 package org.apache.daffodil.processors
 
+import scala.collection.mutable
+
+import org.apache.daffodil.exceptions.Assert
 import org.apache.daffodil.processors.dfa.DFADelimiter
 import org.apache.daffodil.processors.parsers.DelimiterTextType
-import scala.collection.mutable
 import org.apache.daffodil.util.MStackOfInt
 
-trait LocalDelimiters { this: DelimiterIterator =>
-  val delimiterIndexStack: MStackOfInt
-
+trait DelimitersRangeLocal { this: DelimiterIterator =>
   override def reset() {
-    currentIndex = delimiterIndexStack.top - 1
+    currentIndex = this.firstLocalIndex - 1
     indexLimit = this.delimiters.length
   }
 }
 
-trait RemoteDelimiters { this: DelimiterIterator =>
-  val delimiterIndexStack: MStackOfInt
-
+trait DelmitersRangeRemote { this: DelimiterIterator =>
   override def reset() {
     currentIndex = -1
-    indexLimit = delimiterIndexStack.top
+    indexLimit = this.firstLocalIndex
   }
 }
 
-trait RemoteAndLocalDelimiters { this: DelimiterIterator =>
+trait DelimitersRangeAll { this: DelimiterIterator =>
   override def reset() {
     currentIndex = -1
     indexLimit = this.delimiters.length
   }
 }
 
-trait TypedDelimiter { this: DelimiterIterator =>
-  protected def delimType: DelimiterTextType.Type
-
-  override def isOfInterest(delim: DFADelimiter): Boolean = {
-    delim.delimType == delimType
-  }
-}
-
-trait TerminatingMarkup { this: DelimiterIterator =>
-  override def isOfInterest(delim: DFADelimiter) = {
-    delim.delimType == DelimiterTextType.Terminator || delim.delimType == DelimiterTextType.Separator
-  }
-}
-
-trait Separators { this: DelimiterIterator =>
-  @inline
-  override def isOfInterest(delim: DFADelimiter) = {
-    delim.delimType == DelimiterTextType.Separator
-  }
-}
-
-trait Initiators { this: DelimiterIterator =>
-  @inline
-  override def isOfInterest(delim: DFADelimiter) = {
-    delim.delimType == DelimiterTextType.Initiator
-  }
-}
-
-trait Terminators { this: DelimiterIterator =>
-  @inline
-  override def isOfInterest(delim: DFADelimiter) = {
-    delim.delimType == DelimiterTextType.Terminator
-  }
-}
-
 abstract class DelimiterIterator(val delimiters: mutable.ArrayBuffer[DFADelimiter]) {
 
   protected def isOfInterest(delim: DFADelimiter): Boolean
+  def firstLocalIndex: Int
   def reset(): Unit
 
   var currentIndex = -1
   var indexLimit = -1
 
-  // TODO: I'm a little concernece that this doesn't follow standard iterator
-  // practicies. For exmaple, if you called di.next() twice in a row without
-  // calling hasNext, the second next() call would give you the same delimiter.
-  // So for this to work, you really do need to call hasNext before ever
-  // calling next. Although one should follow that practice with iterators, it
-  // isn't usually required.
+  // Note that this does not follow standard iterator practicies. hasNext is
+  // what moves the iterator to the next position, not next(). next() simply
+  // returns the current index in the iterator. So calling next() twice in a
+  // row will return the same thing. So it's important to call hasNext, but
+  // that's always how this is used.
 
 Review comment:
   Should be scaladoc I think. 

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


With regards,
Apache Git Services