CodeQL 8: perf: replace ContainsKey+indexer with TryGetValue

[rlorenzo]

Summary

Closes ~14 of 18 cs/inefficient-containskey alerts. Two distinct cleanups bundled together:

1. C# perf swap — replace ContainsKey + indexer (two dictionary lookups) with TryGetValue (one lookup) at 4 call-sites.
2. Razor dead-code removal — drop the redundant ContainsKey guard in 4 views; ViewData["X"] ?? false already handles missing keys.

What's actually a perf change

dict.ContainsKey(k) + dict[k] does two hash computations and two bucket walks. TryGetValue does one — roughly 2× cheaper per access. String-key sites get the biggest gain because string.GetHashCode() walks every character.

Site	Before	After
RotationsController.cs:498-511	6 lookups per row (3× ContainsKey + 3× indexer) inside a .Select projection	1 TryGetValue per row
RAPSAuditService.cs:144	2 lookups per audit-log entry	1
VMACSExport.cs:302	2 lookups per user + double user.MothraId.Trim()	1 lookup + 1 Trim()
VMACSExport.cs:344 (GetServerUrl)	2 lookups per call	1

The Rotations site is the headline: 6× reduction in dictionary operations per row in the per-week projection. Absolute wall-clock impact is on the order of nanoseconds per lookup, so this is micro-optimization, not something a profiler will flag — but it's free given the alert is firing anyway.

Secondary benefit: TryGetValue is a single atomic lookup, so it's TOCTOU-safe against concurrent dictionary mutation. Not load-bearing for these specific callsites (the dicts aren't shared cross-thread), but it's a strictly safer pattern.

What's NOT a perf change (despite the PR label)

The Razor changes are simplification, not perf:

• ViewData.ContainsKey(\"X\") && (bool)(ViewData[\"X\"] ?? false) → (bool)(ViewData[\"X\"] ?? false)
• ViewData[\"missingKey\"] returns null, not an exception, so the ContainsKey guard was always redundant once the ?? false is there.

Touches Areas/RAPS/Views/Members/List.cshtml (×4), Members/Roles.cshtml (×1), Roles/Members.cshtml (×1), Roles/Templates.cshtml (×4). Also strips the UTF-8 BOM from those four files — encoding hygiene, zero runtime impact.

Not addressed: 1 site in test/Students/EmergencyContactControllerTests.cs:319 (test infrastructure) and ~3 likely already covered or out-of-scope.

Test plan

• npm run test:backend — 1946 tests passing
• Pre-commit lint+test+verify all passed
• CodeQL workflow on this PR shows ~14 of the 18 ContainsKey alerts closed

[coderabbitai]

📝 Walkthrough

Walkthrough

Six files refactored to replace ContainsKey + dictionary indexing with TryGetValue in C# services, and simplified ViewData conditionals in Razor views using null-coalescing operators. UTF-8 BOM markers removed from view file headers.

Changes

Dictionary and ViewData access pattern consolidation

Layer / File(s)	Summary
Service-side dictionary pattern refactoring web/Areas/ClinicalScheduler/Controllers/RotationsController.cs, web/Areas/RAPS/Services/RAPSAuditService.cs, web/Areas/RAPS/Services/VMACSExport.cs	RotationsController.BuildWeekScheduleItem uses TryGetValue for instructor person lookups. RAPSAuditService.GetMemberRolesAndPermissionHistory retrieves action history via TryGetValue. VMACSExport.GetExportUsers and GetServerUrl consolidate permission and server URL lookups into single TryGetValue calls. All changes eliminate redundant ContainsKey checks while preserving fallback defaults.
View-side ViewData conditional simplification web/Areas/RAPS/Views/Members/List.cshtml, web/Areas/RAPS/Views/Members/Roles.cshtml, web/Areas/RAPS/Views/Roles/Members.cshtml, web/Areas/RAPS/Views/Roles/Templates.cshtml	Five Razor view files remove UTF-8 BOM markers from file headers. Conditional rendering logic for permission flags (CanRSOP, canEditRoleMembership, canEditPermissions, canEditRoleTemplates, canApplyTemplates, canViewHistory) simplified from ViewData.ContainsKey(...) && (bool)(ViewData[...] ?? false) to direct (bool)(ViewData["..."] ?? false) checks.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and accurately describes the main change: replacing inefficient ContainsKey+indexer patterns with TryGetValue across multiple files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description clearly relates to the changeset, explaining the ContainsKey+indexer to TryGetValue replacements and Razor view simplifications across all modified files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codeql/8-trygetvalue

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov-commenter]

Bundle Report

Bundle size has no change ✅

[codecov-commenter]

Codecov Report

❌ Patch coverage is 0% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.02%. Comparing base (f45f6b7) to head (0aa8b69).

Files with missing lines	Patch %	Lines
...inicalScheduler/Controllers/RotationsController.cs	0.00%	11 Missing ⚠️
web/Areas/RAPS/Views/Members/List.cshtml	0.00%	4 Missing ⚠️
web/Areas/RAPS/Views/Roles/Templates.cshtml	0.00%	4 Missing ⚠️
web/Areas/RAPS/Services/VMACSExport.cs	0.00%	3 Missing ⚠️
web/Areas/RAPS/Services/RAPSAuditService.cs	0.00%	1 Missing ⚠️
web/Areas/RAPS/Views/Members/Roles.cshtml	0.00%	1 Missing ⚠️
web/Areas/RAPS/Views/Roles/Members.cshtml	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #196      +/-   ##
==========================================
- Coverage   43.02%   43.02%   -0.01%     
==========================================
  Files         881      881              
  Lines       51436    51437       +1     
  Branches     4812     4812              
==========================================
  Hits        22131    22131              
- Misses      28779    28780       +1     
  Partials      526      526              

Flag	Coverage Δ
backend	43.10% <0.00%> (-0.01%)	⬇️
frontend	41.47% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rlorenzo]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[rlorenzo]

@bsedwards Straightforward replacement of ContainsKey with TryGetValue:

https://stackoverflow.com/questions/9382681/what-is-more-efficient-dictionary-trygetvalue-or-containskeyitem