Lucene - Core
  1. Lucene - Core
  2. LUCENE-2315

AttributeSource's methods for accessing attributes should be final, else its easy to corrupt the internal states

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.9, 2.9.1, 2.9.2, 3.0, 3.0.1
    • Fix Version/s: 4.0-ALPHA
    • Component/s: modules/analysis
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      The methods that operate and modify the internal maps of AttributeSource should be final, which is a backwards break. But anybody that overrides such methods simply creates a buggy AS either case.

      I want to makeall impls final (in general the class should be final at all, but it is made for extension in TokenStream). So its important that the implementations are final!

        Activity

        Hide
        Shai Erera added a comment -

        in general the class should be final at all

        How can AttributeSource be final? We want people to develop their own AttributeSources no? Can you please list the methods that you want to make final? I want to check that none of our AttributeSources override them.

        Show
        Shai Erera added a comment - in general the class should be final at all How can AttributeSource be final? We want people to develop their own AttributeSources no? Can you please list the methods that you want to make final? I want to check that none of our AttributeSources override them.
        Hide
        Uwe Schindler added a comment -

        How can AttributeSource be final?

        This was just a comment about the class, but its not possible because it is extended by TokenStreams or similar classes - but the implementation of methods should not be alterable. So all methods should be final, at least all methods that access/modify the private maps.

        A correct plan for "own implementations of AttributeSource" would be to create an abstract AttributeSource base class that defines the behaviour and all impls in the current AttributeSource are final. Because there may be other implementations that work without maps or have a hardcoded number of attributes with optimized implementations.

        Show
        Uwe Schindler added a comment - How can AttributeSource be final? This was just a comment about the class, but its not possible because it is extended by TokenStreams or similar classes - but the implementation of methods should not be alterable. So all methods should be final, at least all methods that access/modify the private maps. A correct plan for "own implementations of AttributeSource" would be to create an abstract AttributeSource base class that defines the behaviour and all impls in the current AttributeSource are final. Because there may be other implementations that work without maps or have a hardcoded number of attributes with optimized implementations.
        Hide
        Shai Erera added a comment -

        Ok I see. I think that instead of creating another class to introduce new users to, we can stick w/ AS and make all the methods that no one shouldn't have any reason to ever extend final. We can keep the methods that define the 'behavior' not final, though I don't see any at the moment. Maybe getAttributeImplsIterator.

        But if will make sense to factor out just these methods to a separate class, so that custom AS don't need to be a sub-class of AS for just that purpose, then I think it'll also be ok.

        Show
        Shai Erera added a comment - Ok I see. I think that instead of creating another class to introduce new users to, we can stick w/ AS and make all the methods that no one shouldn't have any reason to ever extend final. We can keep the methods that define the 'behavior' not final, though I don't see any at the moment. Maybe getAttributeImplsIterator. But if will make sense to factor out just these methods to a separate class, so that custom AS don't need to be a sub-class of AS for just that purpose, then I think it'll also be ok.
        Hide
        Uwe Schindler added a comment -

        This issue was solved by other commits to trunk (e.g. reflection of AttributeSource), the last methods were made final in revision: 1075065

        Show
        Uwe Schindler added a comment - This issue was solved by other commits to trunk (e.g. reflection of AttributeSource), the last methods were made final in revision: 1075065

          People

          • Assignee:
            Uwe Schindler
            Reporter:
            Uwe Schindler
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development