Neha123291 opened a new pull request, #657:
URL: https://github.com/apache/commons-text/pull/657
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
Thanks for your contribution to [Apache
Commons](https://commons.apache.org/)! Your help is appreciated!
Before you push a pull request, review this list:
- [ ] Read the [contribution guidelines](CONTRIBUTING.md) for this project.
- [ ] Run a successful build using the default
[Maven](https://maven.apache.org/) goal with `mvn`; that's `mvn` on the command
line by itself.
- [ ] Write unit tests that match behavioral changes, where the tests fail
if the changes to the runtime are not applied. This may not always be possible
but is a best-practice.
- [ ] Write a pull request description that is detailed enough to understand
what the pull request does, how, and why.
- [ ] Each commit in the pull request should have a meaningful subject line
and body. Note that commits might be squashed by a maintainer on merge.
Commit 1- [Implementation Smell-1: Magic Number
Resolved](https://github.com/apache/commons-text/commit/c2f1ce1c22544d9c991e0116d681500080d518ce)
Description-
The lookup() method in the FileStringLookup class contains a magic number
literal (2) used as part of a conditional check to validate the format of the
input key. However, using the raw value 2 directly in the condition reduces
code readability and makes the logic difficult to understand or modify without
external context. Therefore, Resolved this in this commit for better
readability.
Commit 2- [Implementation Smell-2: Resolved Complex
conditional](https://github.com/apache/commons-text/commit/ebf1c34da8422fa80fbdd5224ae4f2aafa62013a)
Description- The method `translate` in ` NumericEntityUnescaper` contains
complex conditional expression. The Expression is -
input.charAt(index) == '&' && index < seqEnd - 2 && input.charAt(index + 1)
== '#'
Therefore, to simplify it, the logic was broken down into smaller, clearly
named helper methods using Extract Method and Introduce Explaining Variable
techniques. Conditions such as checking if the numeric entity starts correctly,
identifying hexadecimal formats, and verifying the presence of a semicolon were
moved to separate methods like isNumericEntityStart, isHexStart, and
hasSemicolon.
Commit 3- [Implementation smell-3: Resolved Cognitive
complexity](https://github.com/apache/commons-text/commit/aa07a148a6ca1f5547bd54b011295bb50e7eb20c)
Description- The method `lastIndexOf` in ` TextStringBuilder` class has
cognitive complexity. This method has conditions, nested loops, and checks.
Hence, It became overly complex. The refactored lastIndexOf method reduces
cognitive complexity by breaking down the original logic into smaller,
well-named helper methods. Using Extract Method, repetitive and complex
logic—like index adjustment, input validation, and the reverse substring
search—was separated into adjustStartIndex, isInvalidInput, isSearchable, and
searchFromEnd. Introduce Explaining Variable helps clarify the purpose of
computed values, such as the adjusted start index. Additionally, Decompose
Conditional is applied in searchFromEnd to clearly express the nested loop
logic for matching characters from the end. This refactoring improves
readability, testability, and maintainability by making each step of the logic
explicit and modular.
Commit 4- [Design smell-1: Resolved unnecessary abstraction in
StringEscapeUtils…](https://github.com/apache/commons-text/commit/1f25a84d94f53f75ae74cd548cb3834190d72779)
Description-
The XsiUnescaper class adds complexity without improving reusability,
extensibility, or separation of concerns. It is tightly coupled to
StringEscapeUtils and used exclusively to define UNESCAPE_XSI. Since
XsiUnescaper is not reused elsewhere and is not designed for extension, the
abstraction does not serve modular benefits. First, the complex logic inside
the translate method was decomposed by extracting the logic of finding and
writing segments between backslashes into a new helper method, improving
clarity and reducing method length. Next, meaningful names like segmentStart,
searchOffset, and pos were retained to maintain context, while their usage was
clarified through comments or restructuring. Additionally, the BACKSLASH
constant was kept as-is to preserve semantic clarity. These changes make the
code easier to understand and maintain while preserving its original behaviour
and logic.
Commit 5- [Design Smell-2: Resolved SRP Violation in FormattableUtils
Class](https://github.com/apache/commons-text/commit/cc76b76d16d8ae5dda8092a69bf2ebdd57e5d08d)
Description- The Class `FormattableUtils` is God class which violates SRP
(Single responsibility principle). To Resolve this design smell, firstly, I
created a new helper class, namely `TruncatePadFormatter`, which will handle
the logic of precision, ellipsis, and padding. It keeps `FormattableUtils`
focused only on utility orchestration. The class `FormattableUtils` delegates
padding/truncation logic to TruncatePadFormatter and moved logic-heavy append()
method body into the newly created class. This solution also introduced
PadStrategy with “LeftPadder” and “RightPadder”. This solution will improve
code structure.
Commit 6- [Design smell-3: resolved feature envy in StringSubstitutorReader
in
d…](https://github.com/apache/commons-text/commit/b481d0cb98843e09b3e685d2819c58112bb986ef)
Description-
The class StringSubstitutorReader in the commons-text library contains a
Feature Envy smell in the method drain(). This method is overly dependent on
the internal details of the TextStringBuilder class. It frequently accesses
buffer.length(), buffer.isEmpty(), buffer.drainChars(), and buffer.size(),
indicating that it operates more on the TextStringBuilder than on its own data,
violating the principle of object-oriented encapsulation.
To resolve the Feature Envy smell in the StringSubstitutorReader class,
firstly, by applying the Extract Method technique to separate the logic of
interacting with the TextStringBuilder buffer into smaller, more meaningful
methods, making the code easier to understand and reuse. Next, I used Decompose
Conditional to break down complex if statements into simpler conditional
checks, improving readability and comprehension. Then, I introduced Explaining
Variable to assign meaningful variable names to expressions like
buffer.length() or buffer.isEmpty(). Lastly, I used Rename Method/Variable to
make method and variable names more descriptive of their behaviour in the
class.
Commit 7 - [Updated TruncatedPadFormatter file by adding licese
header](https://github.com/apache/commons-text/commit/d03d8ea97811dddf1fe40e3b773224e718739dc6)
Description- Updated TruncatedPadFormatter by adding license header.
Hope this solution helps you.
Let me know if you need anymore changes to be done. If possible, please
accept this pull request.
Thank you.
--
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: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
