garydgregory commented on a change in pull request #821:
URL: https://github.com/apache/commons-lang/pull/821#discussion_r748281092
##########
File path: src/main/java/org/apache/commons/lang3/text/StrBuilder.java
##########
@@ -3045,14 +3045,14 @@ public void write(final int c) {
/** {@inheritDoc} */
@Override
- public void write(final char[] cbuf) {
- StrBuilder.this.append(cbuf);
+ public void write(final char[] buffer) {
Review comment:
-1: Better to keep the name matching the overridden method from
java.io.Writer, it will be less of a surprise if you know the Writer API.
##########
File path: src/main/java/org/apache/commons/lang3/text/WordUtils.java
##########
@@ -402,8 +402,8 @@ public static String capitalize(final String str) {
* @since 2.1
*/
public static String capitalize(final String str, final char...
delimiters) {
- final int delimLen = delimiters == null ? -1 : delimiters.length;
- if (StringUtils.isEmpty(str) || delimLen == 0) {
+ final int delimitersLength = delimiters == null ? -1 :
delimiters.length;
Review comment:
-1: Subjective from me granted, not but better to read IMO.
##########
File path: src/main/java/org/apache/commons/lang3/text/WordUtils.java
##########
@@ -525,8 +525,8 @@ public static String uncapitalize(final String str) {
* @since 2.1
*/
public static String uncapitalize(final String str, final char...
delimiters) {
- final int delimLen = delimiters == null ? -1 : delimiters.length;
- if (StringUtils.isEmpty(str) || delimLen == 0) {
+ final int delimitersLength = delimiters == null ? -1 :
delimiters.length;
Review comment:
-1: Subjective from me granted, not but better to read IMO.
##########
File path: src/main/java/org/apache/commons/lang3/text/StrBuilder.java
##########
@@ -3045,14 +3045,14 @@ public void write(final int c) {
/** {@inheritDoc} */
@Override
- public void write(final char[] cbuf) {
- StrBuilder.this.append(cbuf);
+ public void write(final char[] buffer) {
+ StrBuilder.this.append(buffer);
}
/** {@inheritDoc} */
@Override
- public void write(final char[] cbuf, final int off, final int len) {
- StrBuilder.this.append(cbuf, off, len);
+ public void write(final char[] buffer, final int off, final int len) {
Review comment:
-1: Better to keep the name matching the overridden method from
java.io.Writer, it will be less of a surprise if you know the Writer API.
##########
File path: src/main/java/org/apache/commons/lang3/LocaleUtils.java
##########
@@ -169,21 +169,21 @@ private static boolean isNumericAreaCode(final String
str) {
if (countryCode == null) {
return Collections.emptyList();
}
- List<Locale> langs = cLanguagesByCountry.get(countryCode);
- if (langs == null) {
- langs = new ArrayList<>();
+ List<Locale> langByCountry = cLanguagesByCountry.get(countryCode);
Review comment:
Sorry, _langs_ was better IMO, mostly because it is _plural_ and is
typed as a List. The new name is singular and therefore confusing (to me).
--
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]
