Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Readability: Import Lucene.Net.Search.Similarites.SimilarityBase statically, where appropriate #694

Closed
NightOwl888 opened this issue Oct 18, 2022 · 0 comments · Fixed by #711
Assignees
Labels
good-first-issue Good for newcomers hacktoberfest-accepted is:enhancement New feature or request up-for-grabs This issue is open to be worked on by anyone

Comments

@NightOwl888
Copy link
Contributor

In several code files in Lucene.Net.Search.Similarities the Log2 method is imported directly in Java so it doesn't have to be qualified.

import static org.apache.lucene.search.similarities.SimilarityBase.log2;

public class BasicModelG extends BasicModel {
  
  /** Sole constructor: parameter-free */
  public BasicModelG() {}

  @Override
  public final float score(BasicStats stats, float tfn) {
    // just like in BE, approximation only holds true when F << N, so we use lambda = F / (N + F)
    double F = stats.getTotalTermFreq() + 1;
    double N = stats.getNumberOfDocuments();
    double lambda = F / (N + F);
    // -log(1 / (lambda + 1)) -> log(lambda + 1)
    return (float)(log2(lambda + 1) + tfn * log2((1 + lambda) / lambda));
  }

  @Override
  public String toString() {
    return "G";
  }
}

However, in .NET we are fully qualifying as SimilarityBase.Log2(), which reduces readability and doesn't align well with the original code.

    public class BasicModelG : BasicModel
    {
        /// <summary>
        /// Sole constructor: parameter-free </summary>
        public BasicModelG()
        {
        }

        public override sealed float Score(BasicStats stats, float tfn)
        {
            // just like in BE, approximation only holds true when F << N, so we use lambda = F / (N + F)
            double F = stats.TotalTermFreq + 1;
            double N = stats.NumberOfDocuments;
            double lambda = F / (N + F);
            // -log(1 / (lambda + 1)) -> log(lambda + 1)
            return (float)(SimilarityBase.Log2(lambda + 1) + tfn * SimilarityBase.Log2((1 + lambda) / lambda));
        }

        public override string ToString()
        {
            return "G";
        }
    }

To fix this, we should import the static members of SimilarityBase in every class where it is used. Add the following line at the top of the file above the namespace declaration.

using static Lucene.Net.Search.Similarities.SimilarityBase;

Then we can remove the qualifying statements for Log2().

    public class BasicModelG : BasicModel
    {
        /// <summary>
        /// Sole constructor: parameter-free </summary>
        public BasicModelG()
        {
        }

        public override sealed float Score(BasicStats stats, float tfn)
        {
            // just like in BE, approximation only holds true when F << N, so we use lambda = F / (N + F)
            double F = stats.TotalTermFreq + 1;
            double N = stats.NumberOfDocuments;
            double lambda = F / (N + F);
            // -log(1 / (lambda + 1)) -> log(lambda + 1)
            return (float)(Log2(lambda + 1) + tfn * Log2((1 + lambda) / lambda));
        }

        public override string ToString()
        {
            return "G";
        }
    }

We should do this in all classes in Lucene.Net.Search.Similarities namespace that use the SimilarityBase.Log2() method.

@NightOwl888 NightOwl888 added up-for-grabs This issue is open to be worked on by anyone good-first-issue Good for newcomers is:enhancement New feature or request hacktoberfest-accepted labels Oct 18, 2022
@NightOwl888 NightOwl888 self-assigned this Oct 20, 2022
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Oct 20, 2022
…ere appropriate so the Log2 method doesn't have to be qualified (like in Lucene). Fixes apache#694.
NightOwl888 added a commit that referenced this issue Oct 20, 2022
…ere appropriate so the Log2 method doesn't have to be qualified (like in Lucene). Fixes #694.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue Good for newcomers hacktoberfest-accepted is:enhancement New feature or request up-for-grabs This issue is open to be worked on by anyone
Projects
None yet
1 participant