[jira] Commented: (UIMA-1523) Generics: FSIterator.moveTo(T) should be moveTo(FeatureStructure)

["Jörn Kottmann (JIRA)" <ui...@incubator.apache.org>]

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


> Generics: FSIterator.moveTo(T) should be moveTo(FeatureStructure)
> -----------------------------------------------------------------
>
>                 Key: UIMA-1523
>                 URL: https://issues.apache.org/jira/browse/UIMA-1523
>             Project: UIMA
>          Issue Type: Bug
>          Components: Core Java Framework
>            Reporter: Jörn Kottmann
>            Assignee: Jörn Kottmann
>             Fix For: 2.3
>
>
> The moveTo method of the FSIterator should take a FeatureStructure as argument and not the type parameter T.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Re: [jira] Commented: (UIMA-1523) Generics: FSIterator.moveTo(T) should be moveTo(FeatureStructure)

[Thilo Goetz <tw...@gmx.de>]

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

Re: [jira] Commented: (UIMA-1523) Generics: FSIterator.moveTo(T) should be moveTo(FeatureStructure)

[Jörn Kottmann <ko...@gmail.com>]

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.

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