perf(dotnet): optimize dictionary lookups in model Equals methods#664
Conversation
Address CodeQL warning about inefficient ContainsKey usage by replacing
ContainsKey + indexer pattern with TryGetValue.
Changes:
- Update modelGeneric.mustache template line 357
- Replace two dictionary lookups with single TryGetValue call
- Applies to all generated models with AdditionalProperties
Before (inefficient - 2 lookups):
input.AdditionalProperties.ContainsKey(kv.Key) &&
Equals(kv.Value, input.AdditionalProperties[kv.Key])
After (efficient - 1 lookup):
input.AdditionalProperties.TryGetValue(kv.Key, out var inputValue) &&
Equals(kv.Value, inputValue)
This resolves the CodeQL warning from dotnet-sdk PR #156 and improves
performance for all model equality comparisons across the SDK.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA .NET template file was modified to refactor the Equals method's AdditionalProperties comparison logic. The implementation changed from using ContainsKey and key lookup to using TryGetValue within a lambda expression for retrieving and comparing property values. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
Address two CodeQL warnings in generated model Equals methods: 1. Inefficient ContainsKey usage - Replace ContainsKey + indexer with TryGetValue - Reduces dictionary lookups from 2 to 1 per key 2. Equals should not apply "as" cast - Add GetType() check before casting - Ensures proper equality for subclasses Changes: - Line 320-321: Fix Equals(object) to use GetType() check - Line 357: Optimize AdditionalProperties comparison with TryGetValue These fixes apply to all generated models with AdditionalProperties. Resolves CodeQL warnings from dotnet-sdk PR #156.
Optimize Dictionary Lookups in Model Equals Methods
Problem
The generated model Equals methods perform inefficient dictionary comparisons for
AdditionalProperties:This performs two dictionary lookups per key:
ContainsKey(kv.Key)- checks if key existsinput.AdditionalProperties[kv.Key]- retrieves the valueCodeQL Warning
Source: CodeQL static analysis on openfga/dotnet-sdk#156
Warning: "Inefficient use of ContainsKey and indexer"
Impact: Performance penalty on every equality check for models with AdditionalProperties
Solution
Use
TryGetValuewhich performs both operations in a single dictionary lookup:Changes
File:
config/clients/dotnet/template/modelGeneric.mustacheline 357Before:
After:
Impact
This optimization applies to all generated models with AdditionalProperties, including:
Testing
Verified that the generated code:
References
TryGetValueinstead ofContainsKey+ indexerFiles Modified
config/clients/dotnet/template/modelGeneric.mustacheSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.