Lucene - Core
  1. Lucene - Core
  2. LUCENE-5280

Rename TermFreqPayloadIterator -> SuggestionIterator

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.6, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      The current name is cumbersome, and annoying to change whenever we add something to the iterator (in this case payloads). Since we are breaking it anyway in 4.6, I think we should take the opportunity to find a better name.

      1. LUCENE-5280.patch
        139 kB
        Areek Zillur

        Activity

        Hide
        Areek Zillur added a comment - - edited

        I was wondering if "SuggestionInputIterator" would be a better name? At least to me SuggestionIterator seems to imply the iterator for List<LookupResult> rather than the iterator that gets consumed by all the suggesters. Though I have to admit SuggestionIterator sounds a lot nicer than SuggestionInputIterator

        Show
        Areek Zillur added a comment - - edited I was wondering if "SuggestionInputIterator" would be a better name? At least to me SuggestionIterator seems to imply the iterator for List<LookupResult> rather than the iterator that gets consumed by all the suggesters. Though I have to admit SuggestionIterator sounds a lot nicer than SuggestionInputIterator
        Hide
        Robert Muir added a comment -

        I have two questions: what in the .spell package really uses this interface? Shouldn't it go into suggest?

        If HighFrequencyDictionary still wants to "use it" thats fine I think, but nothing in .spell cares about any of these iterators except for that they are BytesRefIterator (they just want to consume terms): I think it belongs in suggest.

        So if we agree it belongs there, I think InputIterator is then a good name? it would be org.apache.lucene.suggest.InputIterator...

        Show
        Robert Muir added a comment - I have two questions: what in the .spell package really uses this interface? Shouldn't it go into suggest? If HighFrequencyDictionary still wants to "use it" thats fine I think, but nothing in .spell cares about any of these iterators except for that they are BytesRefIterator (they just want to consume terms): I think it belongs in suggest. So if we agree it belongs there, I think InputIterator is then a good name? it would be org.apache.lucene.suggest.InputIterator...
        Hide
        Areek Zillur added a comment -

        it would be org.apache.lucene.suggest.InputIterator...

        That would solve all the confusion!

        Show
        Areek Zillur added a comment - it would be org.apache.lucene.suggest.InputIterator... That would solve all the confusion!
        Hide
        Robert Muir added a comment -

        I would also want to fix the following:

        • BufferingTermFreqPayloadIteratorWrapper
        • SortedTermFreqPayloadIteratorWrapper
        • UnsortedTermFreqPayloadIteratorWrapper.java

        These names are too long: instead we could simpler names like BufferedInputIterator. This would be consistent with JDK classes such as BufferedReader or BufferedInputStream: should be -ed, not -ing, and doesnt need -Wrapper either.

        Show
        Robert Muir added a comment - I would also want to fix the following: BufferingTermFreqPayloadIteratorWrapper SortedTermFreqPayloadIteratorWrapper UnsortedTermFreqPayloadIteratorWrapper.java These names are too long: instead we could simpler names like BufferedInputIterator. This would be consistent with JDK classes such as BufferedReader or BufferedInputStream: should be -ed, not -ing, and doesnt need -Wrapper either.
        Hide
        Areek Zillur added a comment - - edited

        Given Michael agrees with the above discussion, here is the patch (no functional changes):

        • renamed TermFreqPayloadIterator -> InputIterator (moved from .spell to .suggest)
        • renamed BufferingTermFreqPayloadIteratorWrapper -> BufferedInputIterator
        • renamed UnsortedTermFreqPayloadIteratorWrapper -> UnsortedInputIterator
        • renamed SortedTermFreqPayloadIteratorWrapper -> SortedInputIterator
        • renamed TermWeightPayloadIterator -> DocumentInputIterator (in DocumentDictionary)
        • Got rid of unused imports in DocumentDictionary
        • renamed WFSTTermFreqIteratorWrapper -> WFSTInputIterator (in WFSTCompletionLookup)
        • (in tests) renamed TermFreqPayload -> Input
        • (in tests) renamed TermFreqPayloadArrayIterator -> InputArrayIterator
        • renamed TestTermFreqPayloadIterator -> TestInputIterator
        Show
        Areek Zillur added a comment - - edited Given Michael agrees with the above discussion, here is the patch (no functional changes): renamed TermFreqPayloadIterator -> InputIterator (moved from .spell to .suggest) renamed BufferingTermFreqPayloadIteratorWrapper -> BufferedInputIterator renamed UnsortedTermFreqPayloadIteratorWrapper -> UnsortedInputIterator renamed SortedTermFreqPayloadIteratorWrapper -> SortedInputIterator renamed TermWeightPayloadIterator -> DocumentInputIterator (in DocumentDictionary) Got rid of unused imports in DocumentDictionary renamed WFSTTermFreqIteratorWrapper -> WFSTInputIterator (in WFSTCompletionLookup) (in tests) renamed TermFreqPayload -> Input (in tests) renamed TermFreqPayloadArrayIterator -> InputArrayIterator renamed TestTermFreqPayloadIterator -> TestInputIterator
        Hide
        Michael McCandless added a comment -

        +1 for InputIterator

        Show
        Michael McCandless added a comment - +1 for InputIterator
        Hide
        Michael McCandless added a comment -

        Patch looks great, I'll commit shortly. Thanks Areek!

        Show
        Michael McCandless added a comment - Patch looks great, I'll commit shortly. Thanks Areek!
        Hide
        ASF subversion and git services added a comment -

        Commit 1531987 from Michael McCandless in branch 'dev/trunk'
        [ https://svn.apache.org/r1531987 ]

        LUCENE-5280: rename TermFreqPayloadIterator -> InputIterator

        Show
        ASF subversion and git services added a comment - Commit 1531987 from Michael McCandless in branch 'dev/trunk' [ https://svn.apache.org/r1531987 ] LUCENE-5280 : rename TermFreqPayloadIterator -> InputIterator
        Hide
        ASF subversion and git services added a comment -

        Commit 1532001 from Michael McCandless in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1532001 ]

        LUCENE-5280: rename TermFreqPayloadIterator -> InputIterator

        Show
        ASF subversion and git services added a comment - Commit 1532001 from Michael McCandless in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1532001 ] LUCENE-5280 : rename TermFreqPayloadIterator -> InputIterator
        Hide
        Michael McCandless added a comment -

        Thanks Areek!

        Show
        Michael McCandless added a comment - Thanks Areek!
        Hide
        Areek Zillur added a comment -

        Thanks for committing this!

        Show
        Areek Zillur added a comment - Thanks for committing this!

          People

          • Assignee:
            Michael McCandless
            Reporter:
            Michael McCandless
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development