Task/rdmp 360 catalogue status change behaviour#2314
Task/rdmp 360 catalogue status change behaviour#2314
Conversation
…sk/RDMP-360-catalogue-status-change-behaviour
|
|
||
| public List<Thread> Threads = new(); | ||
| private ICoreChildProvider _coreChildProvider; | ||
| private IBasicActivateItems _activator; |
Check notice
Code scanning / CodeQL
Missed 'readonly' opportunity Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
In general, to fix this class of issue, you add the readonly modifier to private fields that are only ever assigned in their declaration or in constructors of the same class. This ensures that after object construction, the reference cannot be changed, preventing accidental reassignment and clarifying intent.
For this specific case in Rdmp.Core/CohortCreation/Execution/CohortCompiler.cs, the best minimal change is to change the field declaration at line 79 from private IBasicActivateItems _activator; to private readonly IBasicActivateItems _activator;. The constructor already assigns _activator, which is allowed for readonly fields, so no other code changes are necessary. No new methods, imports, or other definitions are required.
| @@ -76,7 +76,7 @@ | ||
|
|
||
| public List<Thread> Threads = new(); | ||
| private ICoreChildProvider _coreChildProvider; | ||
| private IBasicActivateItems _activator; | ||
| private readonly IBasicActivateItems _activator; | ||
|
|
||
| public CohortCompiler(IBasicActivateItems activator, CohortIdentificationConfiguration cohortIdentificationConfiguration) | ||
| { |
| private ExecuteCommandClearQueryCache _clearCacheCommand; | ||
|
|
||
| private CohortIdentificationConfigurationUICommon Common = new(); | ||
| private CohortIdentificationConfigurationUICommon Common = new(null); |
Check notice
Code scanning / CodeQL
Missed 'readonly' opportunity Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix the issue, the field Common should be declared with the readonly modifier so that it cannot be assigned after the object is constructed. C# permits multiple assignments to a readonly field as long as they occur either at the declaration or within any constructor of the same class, which matches the current usage (= new(null) at declaration and Common = new CohortIdentificationConfigurationUICommon(Activator); in the constructor). This change does not alter runtime behavior but does enforce immutability of the reference after construction.
Concretely, in Rdmp.UI/SubComponents/CohortIdentificationConfigurationUI.cs, update the declaration on line 76 from private CohortIdentificationConfigurationUICommon Common = new(null); to private readonly CohortIdentificationConfigurationUICommon Common = new(null);. No other code changes, imports, or new methods are required, since the constructor assignment remains legal for a readonly field and no other uses are affected.
| @@ -73,7 +73,7 @@ | ||
|
|
||
| private ExecuteCommandClearQueryCache _clearCacheCommand; | ||
|
|
||
| private CohortIdentificationConfigurationUICommon Common = new(null); | ||
| private readonly CohortIdentificationConfigurationUICommon Common = new(null); | ||
|
|
||
| public CohortIdentificationConfigurationUI() | ||
| { |
| { | ||
| private readonly IBasicActivateItems _activator; | ||
| private CohortIdentificationConfigurationUICommon Common = new (); | ||
| private CohortIdentificationConfigurationUICommon Common = new (null); |
Check notice
Code scanning / CodeQL
Missed 'readonly' opportunity Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
In general, to fix a “missed readonly opportunity” for a field, you add the readonly modifier to the field declaration, provided that the field’s reference is only assigned at declaration or within constructors of the same class, and never reassigned elsewhere. This preserves existing behavior while preventing accidental reassignment of the field after object construction.
For this specific case, the best fix is to update the declaration of Common in ConsoleGuiCohortIdentificationConfigurationUI so that it becomes private readonly CohortIdentificationConfigurationUICommon Common = new (null);. This change does not alter how Common is used: all current usages mutate the internal state of the CohortIdentificationConfigurationUICommon instance, not the field reference itself, so they remain valid. No additional methods, imports, or definitions are required; it is a one-word modifier change on the existing field declaration line. The change should be made where Common is declared near the top of Tools/rdmp/CommandLine/Gui/ConsoleGuiCohortIdentificationConfigurationUI.cs (line 26 as shown).
| @@ -23,7 +23,7 @@ | ||
| public partial class ConsoleGuiCohortIdentificationConfigurationUI | ||
| { | ||
| private readonly IBasicActivateItems _activator; | ||
| private CohortIdentificationConfigurationUICommon Common = new (null); | ||
| private readonly CohortIdentificationConfigurationUICommon Common = new (null); | ||
| private bool _isDisposed; | ||
| private List<object> RowObjects = new(); | ||
| private bool _contextMenuShowing = false; |
Proposed Change
Stop allowing project-specific catalogues ot be used in CIC that are not associated with that project
Type of change
What types of changes does your code introduce? Tick all that apply.
Checklist
By opening this PR, I confirm that I have: