Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: Lucene.Net 2.9.4, Lucene.Net 2.9.4g, Lucene.Net 3.0.3
    • Fix Version/s: Lucene.Net 3.0.3
    • Labels:
      None
    • Environment:

      all

      Description

      We should use .NET properties where ever possible. There are many methods in the API that use methods similar to Class.Getxxxxx() or Class.Setxxxxx(). These methods often just return a less-accessible field, with no real logic behind it.

      • If there are both public Get/Set methods with no special logic, they can be turned into an automatic property: Name { get; set; }
      • If there are both Get/Set methods with no special logic and the setter is private, use an automatic property: Name { get; private set; }
      • In other cases, use good judgement based with the amount of logic that is present in the getter and setter methods.

        Activity

        Christopher Currens made changes -
        Field Original Value New Value
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Christopher Currens added a comment -

        Thanks for the feedback, Itamar. I'm hoping soon to get feedback from others in the community soon, I think Michael is on his way to start promoting it. I suppose whatever feedback we get from the majority will shape what we decide to do with it.

        That being said, it would be good to record the progress of this issue here. It's so close to being done, there are only a few other methods I'd like to convert to properties. I think in some of the attributes there are methods like public string Term(), which are ideal candidates for property conversion.

        Show
        Christopher Currens added a comment - Thanks for the feedback, Itamar. I'm hoping soon to get feedback from others in the community soon, I think Michael is on his way to start promoting it. I suppose whatever feedback we get from the majority will shape what we decide to do with it. That being said, it would be good to record the progress of this issue here. It's so close to being done, there are only a few other methods I'd like to convert to properties. I think in some of the attributes there are methods like public string Term() , which are ideal candidates for property conversion.
        Hide
        Itamar Syn-Hershko added a comment -

        Yes, I followed it to some extent, although not closely enough.

        I don't want to reopen the discussion here. For me, it was quite comfortable moving between the Java and .NET codebases. Being of the CLucene core team, I can also tell you this has tremendous effect on users and on your ability to keep up with the Java version.

        The looks of a foreach loop doesn't really matter IMO, but function names usually do. This is why I think it should only be done when it brings more performance.

        But as long as you're aware of this and doing this carefully, I'll just let it be.

        Show
        Itamar Syn-Hershko added a comment - Yes, I followed it to some extent, although not closely enough. I don't want to reopen the discussion here. For me, it was quite comfortable moving between the Java and .NET codebases. Being of the CLucene core team, I can also tell you this has tremendous effect on users and on your ability to keep up with the Java version. The looks of a foreach loop doesn't really matter IMO, but function names usually do. This is why I think it should only be done when it brings more performance. But as long as you're aware of this and doing this carefully, I'll just let it be.
        Hide
        Christopher Currens added a comment -

        I understand your concern. We've had a lot of discussion about whether to abandon the Java style API for one more friendly to .NET, one of the more in depth ones where we seemed to finally come to a consensus was here in this mailing list thread and its history.

        Ultimately .NET properties are nothing more than syntactical sugar, since there's no real performance benefit over using a Get/Set method. Either one has just as likely of a change to be inlined than the other. However, it makes Lucene.NET stand out from the rest of the .NET libraries, where .NET features are used (and rightly so).

        I'm not sure that deviation from the original API is a valid reason not to implement this feature (not to mention, it's been partially implemented already), considering the plan is and always has been implementing a newer .NET API. There have been considerations to emulate the Java API on top of the new API in the future for people that want that, but IMO it's not really worth it.

        Either way, I don't want to get into a discussion about whether to stick to the java API here, that's something for the mailing lists. If you have objections to the ultimate goal of the project, it would be best you voice them on the dev mailing list. I don't think our community is as talkative as it could be, and we only get input from non-committers from just a few vocal community members. We'd love to hear your input on our goals.

        Show
        Christopher Currens added a comment - I understand your concern. We've had a lot of discussion about whether to abandon the Java style API for one more friendly to .NET, one of the more in depth ones where we seemed to finally come to a consensus was here in this mailing list thread and its history . Ultimately .NET properties are nothing more than syntactical sugar, since there's no real performance benefit over using a Get/Set method. Either one has just as likely of a change to be inlined than the other. However, it makes Lucene.NET stand out from the rest of the .NET libraries, where .NET features are used (and rightly so). I'm not sure that deviation from the original API is a valid reason not to implement this feature (not to mention, it's been partially implemented already), considering the plan is and always has been implementing a newer .NET API. There have been considerations to emulate the Java API on top of the new API in the future for people that want that, but IMO it's not really worth it. Either way, I don't want to get into a discussion about whether to stick to the java API here, that's something for the mailing lists. If you have objections to the ultimate goal of the project, it would be best you voice them on the dev mailing list. I don't think our community is as talkative as it could be, and we only get input from non-committers from just a few vocal community members. We'd love to hear your input on our goals.
        Hide
        Itamar Syn-Hershko added a comment -

        Considering how that would be a deviation from the original API, I would recommend not doing so

        .NET properties are nothing special, so this won't bring any added value, only confusion

        Show
        Itamar Syn-Hershko added a comment - Considering how that would be a deviation from the original API, I would recommend not doing so .NET properties are nothing special, so this won't bring any added value, only confusion
        Christopher Currens created issue -

          People

          • Assignee:
            Unassigned
            Reporter:
            Christopher Currens
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development