Refactor METAR/TAF entrypoints to reduce complexity#80
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Qodana Community for .NET4 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked View the detailed Qodana reportTo be able to view the detailed Qodana report, you can either:
To get - name: 'Qodana Scan'
uses: JetBrains/[email protected]
with:
upload-result: trueContact Qodana teamContact us at [email protected]
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| private static string PrepareRemainingTaf(string cleanTaf) | ||
| { | ||
| if (cleanTaf.Contains("CNL")) | ||
| { | ||
| remainingTaf = cleanTaf; | ||
| return cleanTaf; | ||
| } | ||
|
|
||
| var decodedTaf = new DecodedTaf(cleanTaf); | ||
| var withCavok = false; | ||
| return cleanTaf.Trim() + " END"; | ||
| } |
There was a problem hiding this comment.
📝 Info: Condition inversion in PrepareRemainingTaf is correct but subtle
The old code used if (!cleanTaf.Contains("CNL")) with the "append END" logic in the true branch and the passthrough in the else branch. The new PrepareRemainingTaf at src/Taf.Decoder/TafDecoder.cs:122 inverts this to early-return on Contains("CNL") and fall through to the Trim() + " END" path. This is logically equivalent but worth a second glance since condition inversions are a common source of refactoring bugs. I verified both paths produce identical results.
Was this helpful? React with 👍 or 👎 to provide feedback.
| } | ||
|
|
||
| // weather evolutions | ||
| private static void DecodeEvolutions(DecodedTaf decodedTaf, bool isStrict, bool withCavok, ref string remainingTaf) |
There was a problem hiding this comment.
📝 Info: withCavok passed by value to DecodeEvolutions prevents future propagation
In DecodeEvolutions at src/Taf.Decoder/TafDecoder.cs:158, withCavok is passed by value rather than by ref. This matches the original code's behavior (the old code never modified withCavok after the chunk-decoding loop). However, if a future change needed EvolutionChunkDecoder to update the CAVOK state and have it visible to the caller, this by-value parameter would silently swallow the update. The remainingTaf parameter is correctly passed by ref since it IS modified. This is a minor design consideration for future maintainability.
Was this helpful? React with 👍 or 👎 to provide feedback.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| } | ||
| } | ||
| } |
2 participants
All Submissions:
Changes to Core Features:
ParseWithModestays shallow and Sonar S3776 is reduced.Validation:
dotnet build MetarDecoder.sln --configuration Releasedotnet test tests/Metar.Decoder.Tests/Metar.Decoder.Tests.csproj --framework net8.0 --configuration Release --no-builddotnet test tests/Metar.Decoder.Tests/Metar.Decoder.Tests.csproj --framework net10.0 --configuration Release --no-builddotnet test tests/Taf.Decoder.Tests/Taf.Decoder.Tests.csproj --framework net8.0 --configuration Release --no-builddotnet test tests/Taf.Decoder.Tests/Taf.Decoder.Tests.csproj --framework net10.0 --configuration Release --no-buildNote:
dotnet test MetarDecoder.sln --configuration Release --collect:"XPlat Code Coverage"aborts on Linux when it reaches the net48 test targets becausemonois not installed in this environment; the supported net8.0 and net10.0 test targets passed.Link to Devin session: https://app.devin.ai/sessions/2e7a88c1ff514e9584eb9781f958c070