Refactor LociInWindowsIterator

Replace LociInWindowsIterator

* Drops the LociInWindowsIterator class, adds the SlidingWindow.advanceMultiple function as a replacement, and modifies DistributedUtil to use this function.
* To enable this implementation, adds an iterator class to LociSet.SingleContig.
* Fixes a bug in LociMap I hit while working on this.

The primary motivation for this is readability, but there is also a subtle bug fix here.  The hasNext() function on LociInWindowsIterator wasn't correct in general, since if skipEmpty=true, then it is impossible to know whether there are really more regions available to advance to without actually doing the advancing. (For example, suppose we have a window at locus 100, the next region to be read by that window is at locus 150, and we want to advance to position 1000. It is impossible to know if there are in fact regions at locus 1000 without first advancing through the region at locus 150.)

Closes #214.
ah, that's a good idea. Pushing a change now that makes it a
BufferedIterator and makes everything else private.

On Mon, Nov 24, 2014 at 4:14 PM, Arun Ahuja <notifications@github.com>
wrote:

> In src/main/scala/org/bdgenomics/guacamole/windowing/SlidingWindow.scala:
>
> >     */
> > -  private[windowing] def nextElement: Option[Region] = {
> > -    if (sortedRegions.hasNext) Some(sortedRegions.head) else None
> > +  def advanceMultipleWindows[Region <: HasReferenceRegion](windows: PerSample[SlidingWindow[Region]],
> > +                                                           loci: LociSet.SingleContig.Iterator,
> > +                                                           skipEmpty: Boolean = true): Option[Long] = {
> > +    if (skipEmpty) {
> > +      while (loci.hasNext) {
> > +        val nextNonEmptyLocus = windows.flatMap(_.nextLocusWithRegions).reduceOption(_ min _)
> > +        if (nextNonEmptyLocus.isEmpty) {
> > +          // Our windows are out of regions. We're done. Skip the loci iterator forward to the end, so the loop
> > +          // terminates.
> > +          loci.skipToEnd()
> > +        } else if (nextNonEmptyLocus.get <= loci.headOption.get) {
>
> Makes sense, thanks for the explanation - worth making it BufferedIterator
> explicitly, maybe making the range handling private?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/hammerlab/guacamole/pull/248/files#r20822683>.
>