Lucene - Core
  1. Lucene - Core
  2. LUCENE-2200

Several final classes have non-overriding protected members

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 3.0
    • Fix Version/s: 4.0-ALPHA
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      Protected member access in final classes, except where a protected method overrides a superclass's protected method, makes little sense. The attached patch converts final classes' protected access on fields to private, removes two final classes' unused protected constructors, and converts one final class's protected final method to private.

      1. LUCENE-2200.patch
        7 kB
        Steve Rowe
      2. LUCENE-2200.patch
        6 kB
        Steve Rowe
      3. LUCENE-2200.patch
        6 kB
        Steve Rowe

        Activity

        Hide
        Steve Rowe added a comment -

        All tests pass with the attached patch applied.

        Show
        Steve Rowe added a comment - All tests pass with the attached patch applied.
        Hide
        Simon Willnauer added a comment -

        Steve, I briefly looked at your patch. Could we make some of the member vars final too?
        The reader in CharReader or the defaultAnalyzer in ShingleAnalyzerWrapper for instance.

        simon

        Show
        Simon Willnauer added a comment - Steve, I briefly looked at your patch. Could we make some of the member vars final too? The reader in CharReader or the defaultAnalyzer in ShingleAnalyzerWrapper for instance. simon
        Hide
        Steve Rowe added a comment -

        Could we make some of the member vars final too?

        Done in the new version of the patch. Note that I didn't try to look in classes other than those already modified in the previous version of the patch for final class member access modification.

        Show
        Steve Rowe added a comment - Could we make some of the member vars final too? Done in the new version of the patch. Note that I didn't try to look in classes other than those already modified in the previous version of the patch for final class member access modification.
        Hide
        Steve Rowe added a comment -

        FYI, all tests pass for me with the new version of the patch applied.

        Show
        Steve Rowe added a comment - FYI, all tests pass for me with the new version of the patch applied.
        Hide
        Robert Muir added a comment -

        all tests pass and patch looks good to me. will commit at the end of the day.

        Show
        Robert Muir added a comment - all tests pass and patch looks good to me. will commit at the end of the day.
        Hide
        Simon Willnauer added a comment -

        Robert, when you commit this make sure you mark the Attributes in EdgeNGramTokenFilter.java final thanks.
        Steve thanks for the patch, such work is always appreciated.

        simon

        Show
        Simon Willnauer added a comment - Robert, when you commit this make sure you mark the Attributes in EdgeNGramTokenFilter.java final thanks. Steve thanks for the patch, such work is always appreciated. simon
        Hide
        Steve Rowe added a comment -

        Robert, when you commit this make sure you mark the Attributes in EdgeNGramTokenFilter.java final thanks.

        Whoops, I missed those - thanks for checking, Simon. (minGram and maxGram can also be final in EdgeNGramTokenFilter.java.)

        I've attached a new patch that includes these changes – all tests pass.

        Show
        Steve Rowe added a comment - Robert, when you commit this make sure you mark the Attributes in EdgeNGramTokenFilter.java final thanks. Whoops, I missed those - thanks for checking, Simon. (minGram and maxGram can also be final in EdgeNGramTokenFilter.java.) I've attached a new patch that includes these changes – all tests pass.
        Hide
        Robert Muir added a comment -

        Thanks Steven!

        Show
        Robert Muir added a comment - Thanks Steven!

          People

          • Assignee:
            Robert Muir
            Reporter:
            Steve Rowe
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development