Skip to content

FileTarget - Reduce code complexity for NaturalStringComparer#6069

Merged
snakefoot merged 1 commit intoNLog:devfrom
snakefoot:reduce_complexity
Jan 2, 2026
Merged

FileTarget - Reduce code complexity for NaturalStringComparer#6069
snakefoot merged 1 commit intoNLog:devfrom
snakefoot:reduce_complexity

Conversation

@snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Jan 2, 2026

No description provided.

@coderabbitai
Copy link

coderabbitai bot commented Jan 2, 2026 •
edited
Loading

Walkthrough

Refactors NaturalStringComparer within BaseFileArchiveHandler by extracting numeric-block comparison into a new private helper method that handles leading-zero skipping, digit-block length measurement, and digit-by-digit comparison; replaces inline numeric comparison logic with calls to that helper.

Changes

Cohort / File(s) Summary
Natural String Comparison Refactoring
src/NLog/Targets/FileArchiveHandlers/BaseFileArchiveHandler.cs		 Added private helper NaturalStringCompareNumericBlocks(string x, ref int x_pos, string y, ref int y_pos); replaced inline numeric-block comparison with calls to the helper; simplified non-digit character comparison using a local cmp variable and short-circuiting.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐇 I hop through strings where numbers hide,
Zeros skipped neatly, digits aligned,
A helper hums softly, tidy and bright,
Ordering tokens from morning to night,
Sorting made simple — a rabbit's delight.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive No pull request description was provided by the author, which makes it impossible to assess whether any description exists or is related to the changeset. Add a brief description explaining the refactoring changes and the rationale for extracting the NaturalStringCompareNumericBlocks helper method.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main objective of the pull request, which is to refactor and reduce complexity in the NaturalStringComparer through method extraction.
✨ Finishing touches
  • 📝 Generate docstrings
📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9dc99a6 and 0fc9c53.

📒 Files selected for processing (1)
  • src/NLog/Targets/FileArchiveHandlers/BaseFileArchiveHandler.cs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/NLog/Targets/FileArchiveHandlers/BaseFileArchiveHandler.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: task-list-completed
  • GitHub Check: task-list-completed

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

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

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 2, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
90.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@snakefoot snakefoot merged commit bb99454 into NLog:dev Jan 2, 2026
5 of 6 checks passed
@snakefoot snakefoot added this to the 6.1.0 milestone Jan 31, 2026
@snakefoot snakefoot changed the title Reduce code complexity for NaturalStringComparer FileTarget - Reduce code complexity for NaturalStringComparer Jan 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

refactoring size/M

Projects

None yet

Milestone

6.1

Development

Successfully merging this pull request may close these issues.

1 participant

@snakefoot