Lucene - Core
  1. Lucene - Core
  2. LUCENE-4897

Add a sugar API for traversing categories.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 4.2
    • Fix Version/s: 4.3, 6.0
    • Component/s: modules/facet
    • Labels:
    • Lucene Fields:
      New, Patch Available

      Description

      Mike McCandless said in lucene-java-user mailing list.
      "Maybe we could add some simple sugar APIs?

      Eg something like Collection<CategoryPath> getChildren(int parentOrd)?
      (Or maybe it returns Iterator<CategoryPath>?)"

      What about Collection<Integer> getChildren(int parentOrd)?
      Integer would be more versatile and can easily be converted to CategoryPath with TaxonomyReader.getPath.

      1. LUCENE-4897.patch
        9 kB
        Shai Erera
      2. LUCENE-4897.patch
        8 kB
        Shai Erera

        Activity

        Hide
        Shai Erera added a comment -

        I think that we should use a primitive iterator, e.g. facet collections have IntIterator interface. And so the method should be something like IntIterator getChildren(int ordinal)?

        Show
        Shai Erera added a comment - I think that we should use a primitive iterator, e.g. facet collections have IntIterator interface. And so the method should be something like IntIterator getChildren(int ordinal) ?
        Hide
        crocket added a comment - - edited

        I don't know much about IntIterator, but I surmise it'll do good enough.

        Show
        crocket added a comment - - edited I don't know much about IntIterator, but I surmise it'll do good enough.
        Hide
        Shai Erera added a comment -

        Added TaxoReader.getChildren(int ordinal) and corresponding test. I also migrated PrintTaxonomyStats to use getChildren, which removed all mentions of ParallelTaxonomyArrays from it.

        Show
        Shai Erera added a comment - Added TaxoReader.getChildren(int ordinal) and corresponding test. I also migrated PrintTaxonomyStats to use getChildren, which removed all mentions of ParallelTaxonomyArrays from it.
        Hide
        Michael McCandless added a comment -

        Shouldn't next() throw NoSuchElementException if child is already INVALID_ORDINAL? It shouldn't ever return INVALID_ORDINAL, right? Ie, caller screwed up and called next w/o calling hasNext first.

        Show
        Michael McCandless added a comment - Shouldn't next() throw NoSuchElementException if child is already INVALID_ORDINAL? It shouldn't ever return INVALID_ORDINAL, right? Ie, caller screwed up and called next w/o calling hasNext first.
        Hide
        Shai Erera added a comment -

        I looked at other IntIterator impls and none throw NoSuchElementException, so I thought it's best to follow. Also, since it returns ordinals, it's kind of ok to return INVALID_ORDINAL. I wished that we had a simple IntIterator interface with only next() for this case...

        I don't mind throwing it though. What do you think?

        Show
        Shai Erera added a comment - I looked at other IntIterator impls and none throw NoSuchElementException, so I thought it's best to follow. Also, since it returns ordinals, it's kind of ok to return INVALID_ORDINAL. I wished that we had a simple IntIterator interface with only next() for this case... I don't mind throwing it though. What do you think?
        Hide
        Michael McCandless added a comment -

        Or we could return a not-Java-iterator, that just has int next() that returns INVALID_ORDINAL when it's done...

        Show
        Michael McCandless added a comment - Or we could return a not-Java-iterator, that just has int next() that returns INVALID_ORDINAL when it's done...
        Hide
        Michael McCandless added a comment -

        Woops, our comments crossed...

        I wished that we had a simple IntIterator interface with only next() for this case...

        +1, I think that's best.

        Show
        Michael McCandless added a comment - Woops, our comments crossed... I wished that we had a simple IntIterator interface with only next() for this case... +1, I think that's best.
        Hide
        Shai Erera added a comment -

        I wanted to avoid introducing another class (facet collections already use this primitive IntIterator), but maybe a ChildrenIterator with next() is simplest. I'll look into it.

        Show
        Shai Erera added a comment - I wanted to avoid introducing another class (facet collections already use this primitive IntIterator), but maybe a ChildrenIterator with next() is simplest. I'll look into it.
        Hide
        Shai Erera added a comment -

        Patch with ChildrenIterator

        Show
        Shai Erera added a comment - Patch with ChildrenIterator
        Hide
        Michael McCandless added a comment -

        +1, thanks Shai!

        Show
        Michael McCandless added a comment - +1, thanks Shai!
        Hide
        Commit Tag Bot added a comment -

        [trunk commit] shaie
        http://svn.apache.org/viewvc?view=revision&revision=1464730

        LUCENE-4897: add a sugar API for traversing categories

        Show
        Commit Tag Bot added a comment - [trunk commit] shaie http://svn.apache.org/viewvc?view=revision&revision=1464730 LUCENE-4897 : add a sugar API for traversing categories
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] shaie
        http://svn.apache.org/viewvc?view=revision&revision=1464743

        LUCENE-4897: add a sugar API for traversing categories

        Show
        Commit Tag Bot added a comment - [branch_4x commit] shaie http://svn.apache.org/viewvc?view=revision&revision=1464743 LUCENE-4897 : add a sugar API for traversing categories
        Hide
        Shai Erera added a comment -

        Committed to trunk and 4x.

        Show
        Shai Erera added a comment - Committed to trunk and 4x.
        Hide
        Uwe Schindler added a comment -

        Closed after release.

        Show
        Uwe Schindler added a comment - Closed after release.

          People

          • Assignee:
            Shai Erera
            Reporter:
            crocket
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development