NightOwl888 commented on PR #692: URL: https://github.com/apache/lucenenet/pull/692#issuecomment-1282347725
Hmm...that is interesting. This goes against the advice to fixing [CA1810](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1810), which was also recommended to us directly by Microsoft engineers: https://github.com/apache/lucenenet/pull/224#issuecomment-469284006. I was testing out using `ref` arguments to attempt to initialize multiple static variables, since some of them are hard to convert from using static constructors to inline initialization, such as: https://github.com/apache/lucenenet/blob/fef018190729c27faa2e77a15b24c4c1b451e803/src/Lucene.Net.Analysis.Common/Analysis/Gl/GalicianStemmer.cs#L32-L44 `ref` arguments are one way to go about fixing that one. Another might be to create a custom holder class to put all of the state into that returned from a method and is set as a static variable containing these values. But that would break up the rest of the flow of the code quite a bit. ```c# private static readonly StepState state = LoadStepState(); private static StepState LoadStepState() { var result = new StepState(); IDictionary<string, Step> steps = Parse(typeof(GalicianStemmer), "galician.rslp"); result.plural = steps["Plural"]; result.unification = steps["Unification"]; result.adverb = steps["Adverb"]; result.augmentative = steps["Augmentative"]; result.noun = steps["Noun"]; result.verb = steps["Verb"]; result.vowel = steps["Vowel"]; return result; } ``` Use the regex `static [A-Z][A-Za-z0-9_]*?\(\)` to find all of the static constructors that remain. There are some that can't be fixed, such as those using encoding providers, but it would be great to try to address some of these, especially for the shared stuff like `SloppyMath` and `PForDeltaDocIdSet`. There is nothing technically wrong with the `LoadStateTableIndex` method, but the scan is not recognizing that the static field is being initialized using a `ref` parameter. Giving it a second look, I can see that the `statetableIndexMaxChar` variable is not accessed after line 324, but the `maxState` and `maxChar` are. So, the alternative to going back to a static constructor would be to break that logic into its own method which would iterate over the array twice during initialization, once to populate `statetableIndex` and once to populate `statetableIndexMaxChar`. Both of these alternatives are marginally less performant than this one. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
