NLogMessageParameterList - Reduce code complexity for IsValidParameterList #333
Conversation
Codecov Report
@@ Coverage Diff @@
## master #333 +/- ##
==========================================
+ Coverage 81.82% 81.98% +0.15%
==========================================
Files 12 12
Lines 1106 1110 +4
Branches 194 195 +1
==========================================
+ Hits 905 910 +5
+ Misses 138 136 -2
- Partials 63 64 +1
Continue to review full report at Codecov.
|
| originalMessageIndex = null; | ||
| bool? firstParameterIsPositional = null; | ||
| isPositional = false; | ||
| string parameterName; |
There was a problem hiding this comment.
Why is this one moved to the outer scope?
There was a problem hiding this comment.
Old c++ habbit. No need to push the same parameter on the stack for every loop-iteration
There was a problem hiding this comment.
In my experience is could cause memory leaks.
There was a problem hiding this comment.
And I think it's a code smell in Sonar, not sure
There was a problem hiding this comment.
Only tried memory leaks in C# with dangling event-handlers or dispose-handles (keeps things alive after lifetime) or dynamic-compiled-code (can never be freed).
| char firstChar = parameterKey[0]; | ||
| if (GetCaptureType(firstChar) != CaptureType.Normal) | ||
| char firstChar = parameterName[0]; | ||
| if (firstChar >= '0' && firstChar <= '9') |
There was a problem hiding this comment.
I think this check is more expensive than before. but maybe not really significant
There was a problem hiding this comment.
Most users will always use CaptureType.Normal. Doing the isPositional-check first makes the code simpler. It will only change performance for those who always use @ in the parameter-tokens.
There was a problem hiding this comment.
The goal of this PR is to reduce the complexity of IsValidParameterList with minimal impact.
|
Any idea if the coverage is really reduced? Or is it a flaky coverage test? |
…rList (improve test coverage)
Have now increased the code-coverage. |
|
Merged! Thanks! |
And micro-optimization for comparison against OriginalFormatPropertyName (skip null-check by using Equals for OriginalFormatPropertyName-object)