uima-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thilo Goetz <twgo...@gmx.de>
Subject Re: [jira] Commented: (UIMA-1523) Generics: FSIterator.moveTo(T) should be moveTo(FeatureStructure)
Date Wed, 26 Aug 2009 14:53:04 GMT
Jörn Kottmann wrote:
> Jörn Kottmann (JIRA) wrote:
>>     [
>> https://issues.apache.org/jira/browse/UIMA-1523?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12747822#action_12747822
>> ]
>> Jörn Kottmann commented on UIMA-1523:
>> -------------------------------------
>>
>> The implementation of moveTo in UnambiguousIteratorImpl and
>> Subiterator must be changed to handle non AnnotationFS
>> FeatureStructures correctly.
>>
>>   
> What should be done in this case ?
> 
> Lets say I have a Subitertor and call moveTo and pass in a
> FeatureStructure object which
> does not have the AnnoationFS type, now the annotation comparator (which
> through generics is limited to AnnotationFS types)
> cannot be used in the binary search to find the insert location of the FS.
> 
> To solve this we could either reconsider the generification of the
> AnnotationComparator or
> add a special case and place the iterator at a fixed position.

I would consider making the iterator invalid.  Currently, what
happens is completely undefined, the iterator will be placed just
anywhere.  That's not nice at all.  One could do something like:

  ...
  private final Comparator<AnnotationFS> getAnnotationComparator(AnnotationFS fs) {
    if (this.annotationComparator == null) {
      this.annotationComparator = new AnnotationComparator<AnnotationFS>(fs.getCAS()
          .getAnnotationIndex());
    }
    return this.annotationComparator;
  }
  ...

  public void moveTo(FeatureStructure fs) {
    if (fs instanceof AnnotationFS) {
      AnnotationFS annot = (AnnotationFS) fs;
      final int found = Collections.binarySearch(this.list, annot,
          getAnnotationComparator(annot));
      if (found >= 0) {
        this.pos = found;
      } else {
        this.pos = (-found) - 1;
      }
    }
    if (isValid()) {
      moveToLast();
      moveToNext();
    }
  }

On the other hand, this only helps for subiterators, because we know
that all and only AnnotationFSs make sense.  So I'm not sure it's
worth improving this just here and leave it undefined for other cases.

--Thilo

> 
> Well I assume we want to take the first option, because thats how it has
> been before.
> To make a bit cleaner I suggest that we rename the AnnotationComparator
> (its an internal protected class) to something
> like FeatureStructureComparator.
> 
> Jörn

Mime
View raw message