Anik1459 opened a new pull request, #758:
URL: https://github.com/apache/commons-text/pull/758

   ## Summary
   
   This PR addresses four code quality issues identified through manual 
inspection
   and static analysis of the codebase.
   
   ---
   
   ## Changes
   
   ### 1. Fix cross-drive test failure on Windows (FileStringLookupTest)
   
   `testFenceRelativeParentTraversal` used `@TempDir` which JUnit resolves to
   the OS temp directory. On Windows, this may be on a different drive (e.g. 
`C:\`)
   than the project (e.g. `E:\`), causing `Path.relativize()` to throw
   `IllegalArgumentException` — unrelated to the code under test.
   
   Fixed by using `target/tempDir` and `target/fence`, which are guaranteed to 
be
   on the same drive as the project. The relative traversal path (`../`) is 
still
   correctly generated and rejected by the fencing lookup as intended.
   
   ### 2. Improve exception diagnostics in ExtendedMessageFormat
   
   Two `IllegalArgumentException`s in `applyPattern()` used the generic message
   `"The validated expression is false"` — a leftover from an inlined
   `Validate.isTrue()` call. This provides no diagnostic value in logs or 
debuggers.
   
   Replaced with specific messages that include the actual vs expected format 
count.
   
   ### 3. Name magic constants in JaroWinklerSimilarity
   
   The algorithm-specific literals `4` (max prefix length) and `0.7d` (Winkler
   threshold) were used inline with no explanation. Extracted to named constants
   `MAX_PREFIX_LENGTH` and `DEFAULT_WINKLER_THRESHOLD`. No behavioural change.
   
   ### 4. Eliminate duplicated loop in WordUtils (Extract Method)
   
   `capitalize()` and `uncapitalize()` shared an identical 28-line tokenisation
   loop differing only by one function call (`Character::toTitleCase` vs
   `Character::toLowerCase`). Extracted to a private `applyWordCaseTransform()`
   helper that accepts an `IntUnaryOperator`. Both public methods now delegate
   with a single line. No public API change.
   
   ---
   
   ## Testing
   
   - `mvn clean test` — all tests pass (0 failures, 0 errors)
   - Checkstyle: 0 violations
   - PMD: no new violations introduced
   - All public APIs remain backwards compatible
   
   ---
   
   ## Notes
   
   `StringSubstitutor.substitute()` (~140 lines, cyclomatic complexity ~25) was
   identified as a Long Method smell but deliberately excluded. Refactoring that
   performance-critical inner loop carries a high regression risk not 
appropriate
   for this change set.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to