Adjust QueryTemplateManager to allow subclasses avoid calling virtual methods from the constructors #829
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NightOwl888
requested changes
Apr 13, 2023
| @@ -34,20 +35,28 @@ namespace Lucene.Net.QueryParsers.Xml | |||
| public class QueryTemplateManager | |||
| { | |||
| private readonly IDictionary<string, XslCompiledTransform> compiledTemplatesCache = new Dictionary<string, XslCompiledTransform>(); // LUCENENET: marked readonly | |||
| private XslCompiledTransform defaultCompiledTemplates; | |||
| // LUCENENET specific - made protected to allow subclasses to access and initialize | |||
| protected XslCompiledTransform m_defaultCompiledTemplates; | |||
There was a problem hiding this comment.
It looks like this was meant to be a "default template" and the user could subclass to add their own template. To do this they would need to:
- Override the
public QueryTemplateManager()constructor. - Load their own template. However they want to.
- Set their own private
XslCompiledTransformto a private variable. - Override
GetQueryAsXmlStringandGetQueryAsDOMto supply their private variable.
Maybe I have missed something, but I don't think there is any advantage to making this variable protected as it is supposed to represent the "default" template, not the user specified one.
There was a problem hiding this comment.
oh ok, so basically no need to make this variable protected. Adjusting now...
NightOwl888
approved these changes
Apr 13, 2023
laimis
deleted the
fix/virtual-calls-from-constructors-querytemplatemanager
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Continuation of fixes with virtual calls being made from constructors. The issue originally reported by SonarCloud scans: https://sonarcloud.io/project/issues?resolved=false&rules=csharpsquid%3AS1699&id=apache_lucenenet and referenced in this issue: #670
This one focuses on queryTemplateManager where we allow subclasses to use default constructor and expose m_defaultCompiledTemplates as protected for them to initialize it when needed.