[GitHub] [commons-lang] verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length …
verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName
refactored to create appropriate length …
URL: https://github.com/apache/commons-lang/pull/446#discussion_r333058664
##
File path: src/main/java/org/apache/commons/lang3/ClassUtils.java
##
@@ -433,48 +440,82 @@ public static String getAbbreviatedName(final Class
cls, final int len) {
* "java.lang.String" 5"j.l.String"
* "java.lang.String"15"j.lang.String"
*
"java.lang.String"30"java.lang.String"
+ *
"org.apache.commons.lang3.ClassUtils"18"o.a.c.l.ClassUtils"
*
- * @param className the className to get the abbreviated name for, may be
{@code null}
- * @param len the desired length of the abbreviated name
- * @return the abbreviated name or an empty string
- * @throws IllegalArgumentException if len = 0
+ *
+ * @param className the className to get the abbreviated name for, may be
{@code null}
+ * @param len the desired length of the abbreviated name
+ * @return the abbreviated name or an empty string if the specified
+ * class name is {@code null} or empty string
+ * @throws IllegalArgumentException if {@code len <= 0}
* @since 3.4
*/
public static String getAbbreviatedName(final String className, final int
len) {
- if (len <= 0) {
-throw new IllegalArgumentException("len must be > 0");
- }
- if (className == null) {
-return StringUtils.EMPTY;
- }
-
- int availableSpace = len;
- final int packageLevels = StringUtils.countMatches(className, '.');
- final String[] output = new String[packageLevels + 1];
- int endIndex = className.length() - 1;
- for (int level = packageLevels; level >= 0; level--) {
-final int startIndex = className.lastIndexOf('.', endIndex);
-final String part = className.substring(startIndex + 1, endIndex + 1);
-availableSpace -= part.length();
-if (level > 0) {
- // all elements except top level require an additional char space
- availableSpace--;
+if (len <= 0) {
+throw new IllegalArgumentException("len must be > 0");
}
-if (level == packageLevels) {
- // ClassName is always complete
- output[level] = part;
-} else {
- if (availableSpace > 0) {
-output[level] = part;
- } else {
-// if no space is left still the first char is used
-output[level] = part.substring(0, 1);
- }
+if (className == null) {
+return StringUtils.EMPTY;
}
-endIndex = startIndex - 1;
- }
- return StringUtils.join(output, '.');
+final char[] abbreviated = className.toCharArray();
+int target = 0;
+int source = 0;
+while (source < abbreviated.length) {
+// copy the next part
+int runAheadTarget = target;
+while (source < abbreviated.length && abbreviated[source] != '.') {
+abbreviated[runAheadTarget++] = abbreviated[source++];
+}
+
+++target;
+if (useFull(runAheadTarget, source, abbreviated.length, len)
+|| target > runAheadTarget) {
+target = runAheadTarget;
+}
+
+// copy the '.' unless it was the last part
+if (source < abbreviated.length) {
+abbreviated[target++] = abbreviated[source++];
+}
+}
+return new String(abbreviated, 0, target);
+}
+
+/**
+ * Decides if the part that was just copied to its destination
+ * location in the work array can be kept as it was copied or must be
+ * abbreviated. It must be kept when the part is the last one, which
+ * is the simple name of the class. In this case the {@code source}
+ * index, from where the characters are copied points one position
+ * after the last character, a.k.a. {@code source ==
+ * originalLength}
+ *
+ * If the part is not the last one then it can be kept
+ * unabridged if the number of the characters copied so far plus
+ * the character that are to be copied is less than or equal to the
+ * desired length.
+ *
+ * @param runAheadTarget the target index (where the characters were
+ * copied to) pointing after the last character
+ * copied when the current part was copied
+ * @param source the source index (where the characters were
+ * copied from) pointing after the last
+ * character copied when the current part was
+ * copied
+ * @param originalLength the original length of the class full name,
+ * which is abbreviated
+ * @param desiredLength the desired length of the abbreviated class
+ *
[GitHub] [commons-lang] verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length …
verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName
refactored to create appropriate length …
URL: https://github.com/apache/commons-lang/pull/446#discussion_r325709633
##
File path: src/main/java/org/apache/commons/lang3/ClassUtils.java
##
@@ -433,48 +440,82 @@ public static String getAbbreviatedName(final Class
cls, final int len) {
* "java.lang.String" 5"j.l.String"
* "java.lang.String"15"j.lang.String"
*
"java.lang.String"30"java.lang.String"
+ *
"org.apache.commons.lang3.ClassUtils"18"o.a.c.l.ClassUtils"
*
- * @param className the className to get the abbreviated name for, may be
{@code null}
- * @param len the desired length of the abbreviated name
- * @return the abbreviated name or an empty string
- * @throws IllegalArgumentException if len = 0
+ *
+ * @param className the className to get the abbreviated name for, may be
{@code null}
+ * @param len the desired length of the abbreviated name
+ * @return the abbreviated name or an empty string if the specified
Review comment:
It is not and it is described in detail in the above text. To be absolutely
correct I added a half-sentence here as well.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-lang] verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length …
verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName
refactored to create appropriate length …
URL: https://github.com/apache/commons-lang/pull/446#discussion_r328881438
##
File path: src/test/java/org/apache/commons/lang3/ClassUtilsTest.java
##
@@ -160,26 +162,59 @@ public void test_getAbbreviatedName_Class() {
assertEquals("", ClassUtils.getAbbreviatedName((Class) null, 1));
assertEquals("j.l.String", ClassUtils.getAbbreviatedName(String.class,
1));
assertEquals("j.l.String", ClassUtils.getAbbreviatedName(String.class,
5));
+assertEquals("o.a.c.l.ClassUtils",
ClassUtils.getAbbreviatedName(ClassUtils.class, 18));
assertEquals("j.lang.String",
ClassUtils.getAbbreviatedName(String.class, 13));
assertEquals("j.lang.String",
ClassUtils.getAbbreviatedName(String.class, 15));
assertEquals("java.lang.String",
ClassUtils.getAbbreviatedName(String.class, 20));
}
+/**
+ * Test that in case the required length is larger than the name and thus
there is no need for any shortening
+ * then the returned string object is the same as the one passed as
argument. Note, however, that this is
+ * tested as an internal implementation detail, but it is not a guaranteed
feature of the implementation.
+ */
@Test
-public void test_getAbbreviatedName_Class_NegativeLen() {
+@DisplayName("When the length hint is longer than the actual length then
the same String object is returned")
+void test_getAbbreviatedName_TooLongHint(){
+final String className = "java.lang.String";
+Assertions.assertSame(className,
ClassUtils.getAbbreviatedName(className, className.length()+1));
+Assertions.assertSame(className,
ClassUtils.getAbbreviatedName(className, className.length()));
+}
+
+@Test
+@DisplayName("When the desired length is negative then exception is
thrown")
+void test_getAbbreviatedName_Class_NegativeLen() {
assertThrows(IllegalArgumentException.class, () ->
ClassUtils.getAbbreviatedName(String.class, -10));
}
@Test
-public void test_getAbbreviatedName_Class_ZeroLen() {
+@DisplayName("When the desired length is zero then exception is thrown")
+void test_getAbbreviatedName_Class_ZeroLen() {
assertThrows(IllegalArgumentException.class, () ->
ClassUtils.getAbbreviatedName(String.class, 0));
}
@Test
-public void test_getAbbreviatedName_String() {
Review comment:
Sorry, but wrong addressee. This is not me, who has changed the visibility,
but JUnit 5.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-lang] verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length …
verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName
refactored to create appropriate length …
URL: https://github.com/apache/commons-lang/pull/446#discussion_r328881438
##
File path: src/test/java/org/apache/commons/lang3/ClassUtilsTest.java
##
@@ -160,26 +162,59 @@ public void test_getAbbreviatedName_Class() {
assertEquals("", ClassUtils.getAbbreviatedName((Class) null, 1));
assertEquals("j.l.String", ClassUtils.getAbbreviatedName(String.class,
1));
assertEquals("j.l.String", ClassUtils.getAbbreviatedName(String.class,
5));
+assertEquals("o.a.c.l.ClassUtils",
ClassUtils.getAbbreviatedName(ClassUtils.class, 18));
assertEquals("j.lang.String",
ClassUtils.getAbbreviatedName(String.class, 13));
assertEquals("j.lang.String",
ClassUtils.getAbbreviatedName(String.class, 15));
assertEquals("java.lang.String",
ClassUtils.getAbbreviatedName(String.class, 20));
}
+/**
+ * Test that in case the required length is larger than the name and thus
there is no need for any shortening
+ * then the returned string object is the same as the one passed as
argument. Note, however, that this is
+ * tested as an internal implementation detail, but it is not a guaranteed
feature of the implementation.
+ */
@Test
-public void test_getAbbreviatedName_Class_NegativeLen() {
+@DisplayName("When the length hint is longer than the actual length then
the same String object is returned")
+void test_getAbbreviatedName_TooLongHint(){
+final String className = "java.lang.String";
+Assertions.assertSame(className,
ClassUtils.getAbbreviatedName(className, className.length()+1));
+Assertions.assertSame(className,
ClassUtils.getAbbreviatedName(className, className.length()));
+}
+
+@Test
+@DisplayName("When the desired length is negative then exception is
thrown")
+void test_getAbbreviatedName_Class_NegativeLen() {
assertThrows(IllegalArgumentException.class, () ->
ClassUtils.getAbbreviatedName(String.class, -10));
}
@Test
-public void test_getAbbreviatedName_Class_ZeroLen() {
+@DisplayName("When the desired length is zero then exception is thrown")
+void test_getAbbreviatedName_Class_ZeroLen() {
assertThrows(IllegalArgumentException.class, () ->
ClassUtils.getAbbreviatedName(String.class, 0));
}
@Test
-public void test_getAbbreviatedName_String() {
Review comment:
Sorry, but wrong addressee. This is not me, who has changed the visibility,
but JUnit 5.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-lang] verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length …
verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName
refactored to create appropriate length …
URL: https://github.com/apache/commons-lang/pull/446#discussion_r328881421
##
File path: src/test/java/org/apache/commons/lang3/ClassUtilsTest.java
##
@@ -160,26 +162,59 @@ public void test_getAbbreviatedName_Class() {
assertEquals("", ClassUtils.getAbbreviatedName((Class) null, 1));
assertEquals("j.l.String", ClassUtils.getAbbreviatedName(String.class,
1));
assertEquals("j.l.String", ClassUtils.getAbbreviatedName(String.class,
5));
+assertEquals("o.a.c.l.ClassUtils",
ClassUtils.getAbbreviatedName(ClassUtils.class, 18));
assertEquals("j.lang.String",
ClassUtils.getAbbreviatedName(String.class, 13));
assertEquals("j.lang.String",
ClassUtils.getAbbreviatedName(String.class, 15));
assertEquals("java.lang.String",
ClassUtils.getAbbreviatedName(String.class, 20));
}
+/**
+ * Test that in case the required length is larger than the name and thus
there is no need for any shortening
+ * then the returned string object is the same as the one passed as
argument. Note, however, that this is
+ * tested as an internal implementation detail, but it is not a guaranteed
feature of the implementation.
+ */
@Test
-public void test_getAbbreviatedName_Class_NegativeLen() {
+@DisplayName("When the length hint is longer than the actual length then
the same String object is returned")
+void test_getAbbreviatedName_TooLongHint(){
+final String className = "java.lang.String";
+Assertions.assertSame(className,
ClassUtils.getAbbreviatedName(className, className.length()+1));
+Assertions.assertSame(className,
ClassUtils.getAbbreviatedName(className, className.length()));
+}
+
+@Test
+@DisplayName("When the desired length is negative then exception is
thrown")
+void test_getAbbreviatedName_Class_NegativeLen() {
assertThrows(IllegalArgumentException.class, () ->
ClassUtils.getAbbreviatedName(String.class, -10));
}
@Test
-public void test_getAbbreviatedName_Class_ZeroLen() {
Review comment:
Sorry, but wrong addressee. This is not me, who has changed the visibility,
but JUnit 5.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-lang] verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length …
verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName
refactored to create appropriate length …
URL: https://github.com/apache/commons-lang/pull/446#discussion_r328774255
##
File path: src/test/java/org/apache/commons/lang3/ClassUtilsTest.java
##
@@ -160,26 +162,59 @@ public void test_getAbbreviatedName_Class() {
assertEquals("", ClassUtils.getAbbreviatedName((Class) null, 1));
assertEquals("j.l.String", ClassUtils.getAbbreviatedName(String.class,
1));
assertEquals("j.l.String", ClassUtils.getAbbreviatedName(String.class,
5));
+assertEquals("o.a.c.l.ClassUtils",
ClassUtils.getAbbreviatedName(ClassUtils.class, 18));
assertEquals("j.lang.String",
ClassUtils.getAbbreviatedName(String.class, 13));
assertEquals("j.lang.String",
ClassUtils.getAbbreviatedName(String.class, 15));
assertEquals("java.lang.String",
ClassUtils.getAbbreviatedName(String.class, 20));
}
+/**
+ * Test that in case the required length is larger than the name and thus
there is no need for any shortening
+ * then the returned string object is the same as the one passed as
argument. Note, however, that this is
+ * tested as an internal implementation detail, but it is not a guaranteed
feature of the implementation.
+ */
@Test
-public void test_getAbbreviatedName_Class_NegativeLen() {
+@DisplayName("When the length hint is longer than the actual length then
the same String object is returned")
+void test_getAbbreviatedName_TooLongHint(){
+final String className = "java.lang.String";
+Assertions.assertSame(className,
ClassUtils.getAbbreviatedName(className, className.length()+1));
+Assertions.assertSame(className,
ClassUtils.getAbbreviatedName(className, className.length()));
+}
+
+@Test
+@DisplayName("When the desired length is negative then exception is
thrown")
+void test_getAbbreviatedName_Class_NegativeLen() {
assertThrows(IllegalArgumentException.class, () ->
ClassUtils.getAbbreviatedName(String.class, -10));
}
@Test
-public void test_getAbbreviatedName_Class_ZeroLen() {
+@DisplayName("When the desired length is zero then exception is thrown")
+void test_getAbbreviatedName_Class_ZeroLen() {
assertThrows(IllegalArgumentException.class, () ->
ClassUtils.getAbbreviatedName(String.class, 0));
}
@Test
-public void test_getAbbreviatedName_String() {
Review comment:
Why?
Since the project uses JUnit 5 the required visibility is package private.
Any access modifier is noise.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-lang] verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length …
verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName
refactored to create appropriate length …
URL: https://github.com/apache/commons-lang/pull/446#discussion_r327104320
##
File path: src/main/java/org/apache/commons/lang3/ClassUtils.java
##
@@ -433,48 +440,83 @@ public static String getAbbreviatedName(final Class
cls, final int len) {
* "java.lang.String" 5"j.l.String"
* "java.lang.String"15"j.lang.String"
*
"java.lang.String"30"java.lang.String"
+ *
"org.apache.commons.lang3.ClassUtils"18"o.a.c.l.ClassUtils"
*
- * @param className the className to get the abbreviated name for, may be
{@code null}
- * @param len the desired length of the abbreviated name
- * @return the abbreviated name or an empty string
- * @throws IllegalArgumentException if len = 0
+ *
+ * @param className the className to get the abbreviated name for, may be
{@code null}
+ * @param len the desired length of the abbreviated name
+ * @return the abbreviated name or an empty string if the specified
+ * class name is {@code null} or empty string. The abbreviated name may be
+ * longer than the desired length if it cannot be abbreviated to the
desired length.
+ * @throws IllegalArgumentException if {@code len <= 0}
* @since 3.4
*/
public static String getAbbreviatedName(final String className, final int
len) {
- if (len <= 0) {
-throw new IllegalArgumentException("len must be > 0");
- }
- if (className == null) {
-return StringUtils.EMPTY;
- }
-
- int availableSpace = len;
- final int packageLevels = StringUtils.countMatches(className, '.');
- final String[] output = new String[packageLevels + 1];
- int endIndex = className.length() - 1;
- for (int level = packageLevels; level >= 0; level--) {
-final int startIndex = className.lastIndexOf('.', endIndex);
-final String part = className.substring(startIndex + 1, endIndex + 1);
-availableSpace -= part.length();
-if (level > 0) {
- // all elements except top level require an additional char space
- availableSpace--;
+if (len <= 0) {
+throw new IllegalArgumentException("len must be > 0");
}
-if (level == packageLevels) {
- // ClassName is always complete
- output[level] = part;
-} else {
- if (availableSpace > 0) {
-output[level] = part;
- } else {
-// if no space is left still the first char is used
-output[level] = part.substring(0, 1);
- }
+if (className == null) {
+return StringUtils.EMPTY;
}
Review comment:
There is no point to add a test. This is not a feature change. This is a
performance optimization. I do not think the method should guarantee as an API
contract that it will return the same String object in this case.
In other cases I totally agree: if there is a change, which is functionality
change then there should be a test also changed.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-lang] verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length …
verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName
refactored to create appropriate length …
URL: https://github.com/apache/commons-lang/pull/446#discussion_r327104320
##
File path: src/main/java/org/apache/commons/lang3/ClassUtils.java
##
@@ -433,48 +440,83 @@ public static String getAbbreviatedName(final Class
cls, final int len) {
* "java.lang.String" 5"j.l.String"
* "java.lang.String"15"j.lang.String"
*
"java.lang.String"30"java.lang.String"
+ *
"org.apache.commons.lang3.ClassUtils"18"o.a.c.l.ClassUtils"
*
- * @param className the className to get the abbreviated name for, may be
{@code null}
- * @param len the desired length of the abbreviated name
- * @return the abbreviated name or an empty string
- * @throws IllegalArgumentException if len = 0
+ *
+ * @param className the className to get the abbreviated name for, may be
{@code null}
+ * @param len the desired length of the abbreviated name
+ * @return the abbreviated name or an empty string if the specified
+ * class name is {@code null} or empty string. The abbreviated name may be
+ * longer than the desired length if it cannot be abbreviated to the
desired length.
+ * @throws IllegalArgumentException if {@code len <= 0}
* @since 3.4
*/
public static String getAbbreviatedName(final String className, final int
len) {
- if (len <= 0) {
-throw new IllegalArgumentException("len must be > 0");
- }
- if (className == null) {
-return StringUtils.EMPTY;
- }
-
- int availableSpace = len;
- final int packageLevels = StringUtils.countMatches(className, '.');
- final String[] output = new String[packageLevels + 1];
- int endIndex = className.length() - 1;
- for (int level = packageLevels; level >= 0; level--) {
-final int startIndex = className.lastIndexOf('.', endIndex);
-final String part = className.substring(startIndex + 1, endIndex + 1);
-availableSpace -= part.length();
-if (level > 0) {
- // all elements except top level require an additional char space
- availableSpace--;
+if (len <= 0) {
+throw new IllegalArgumentException("len must be > 0");
}
-if (level == packageLevels) {
- // ClassName is always complete
- output[level] = part;
-} else {
- if (availableSpace > 0) {
-output[level] = part;
- } else {
-// if no space is left still the first char is used
-output[level] = part.substring(0, 1);
- }
+if (className == null) {
+return StringUtils.EMPTY;
}
Review comment:
There is no point to add a test. This is not a feature change. This is a
performance optimization. I do not think the method should guarantee as an API
contract that it will return the same String object in this case.
In other cases I totally agree: if there is a change, which is functionality
change then there should be a test also changed.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-lang] verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length …
verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName
refactored to create appropriate length …
URL: https://github.com/apache/commons-lang/pull/446#discussion_r325831427
##
File path: src/main/java/org/apache/commons/lang3/ClassUtils.java
##
@@ -433,48 +440,83 @@ public static String getAbbreviatedName(final Class
cls, final int len) {
* "java.lang.String" 5"j.l.String"
* "java.lang.String"15"j.lang.String"
*
"java.lang.String"30"java.lang.String"
+ *
"org.apache.commons.lang3.ClassUtils"18"o.a.c.l.ClassUtils"
*
- * @param className the className to get the abbreviated name for, may be
{@code null}
- * @param len the desired length of the abbreviated name
- * @return the abbreviated name or an empty string
- * @throws IllegalArgumentException if len = 0
+ *
+ * @param className the className to get the abbreviated name for, may be
{@code null}
+ * @param len the desired length of the abbreviated name
+ * @return the abbreviated name or an empty string if the specified
+ * class name is {@code null} or empty string. The abbreviated name may be
+ * longer than the desired length if it cannot be abbreviated to the
desired length.
+ * @throws IllegalArgumentException if {@code len <= 0}
* @since 3.4
*/
public static String getAbbreviatedName(final String className, final int
len) {
- if (len <= 0) {
-throw new IllegalArgumentException("len must be > 0");
- }
- if (className == null) {
-return StringUtils.EMPTY;
- }
-
- int availableSpace = len;
- final int packageLevels = StringUtils.countMatches(className, '.');
- final String[] output = new String[packageLevels + 1];
- int endIndex = className.length() - 1;
- for (int level = packageLevels; level >= 0; level--) {
-final int startIndex = className.lastIndexOf('.', endIndex);
-final String part = className.substring(startIndex + 1, endIndex + 1);
-availableSpace -= part.length();
-if (level > 0) {
- // all elements except top level require an additional char space
- availableSpace--;
+if (len <= 0) {
+throw new IllegalArgumentException("len must be > 0");
}
-if (level == packageLevels) {
- // ClassName is always complete
- output[level] = part;
-} else {
- if (availableSpace > 0) {
-output[level] = part;
- } else {
-// if no space is left still the first char is used
-output[level] = part.substring(0, 1);
- }
+if (className == null) {
+return StringUtils.EMPTY;
}
Review comment:
Makes sense.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-lang] verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length …
verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName
refactored to create appropriate length …
URL: https://github.com/apache/commons-lang/pull/446#discussion_r325830080
##
File path: src/main/java/org/apache/commons/lang3/ClassUtils.java
##
@@ -433,48 +440,83 @@ public static String getAbbreviatedName(final Class
cls, final int len) {
* "java.lang.String" 5"j.l.String"
* "java.lang.String"15"j.lang.String"
*
"java.lang.String"30"java.lang.String"
+ *
"org.apache.commons.lang3.ClassUtils"18"o.a.c.l.ClassUtils"
*
- * @param className the className to get the abbreviated name for, may be
{@code null}
- * @param len the desired length of the abbreviated name
- * @return the abbreviated name or an empty string
- * @throws IllegalArgumentException if len = 0
+ *
+ * @param className the className to get the abbreviated name for, may be
{@code null}
+ * @param len the desired length of the abbreviated name
+ * @return the abbreviated name or an empty string if the specified
+ * class name is {@code null} or empty string. The abbreviated name may be
+ * longer than the desired length if it cannot be abbreviated to the
desired length.
+ * @throws IllegalArgumentException if {@code len <= 0}
* @since 3.4
*/
public static String getAbbreviatedName(final String className, final int
len) {
Review comment:
I can live with that. Makes sense.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-lang] verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length …
verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName
refactored to create appropriate length …
URL: https://github.com/apache/commons-lang/pull/446#discussion_r325709633
##
File path: src/main/java/org/apache/commons/lang3/ClassUtils.java
##
@@ -433,48 +440,82 @@ public static String getAbbreviatedName(final Class
cls, final int len) {
* "java.lang.String" 5"j.l.String"
* "java.lang.String"15"j.lang.String"
*
"java.lang.String"30"java.lang.String"
+ *
"org.apache.commons.lang3.ClassUtils"18"o.a.c.l.ClassUtils"
*
- * @param className the className to get the abbreviated name for, may be
{@code null}
- * @param len the desired length of the abbreviated name
- * @return the abbreviated name or an empty string
- * @throws IllegalArgumentException if len = 0
+ *
+ * @param className the className to get the abbreviated name for, may be
{@code null}
+ * @param len the desired length of the abbreviated name
+ * @return the abbreviated name or an empty string if the specified
Review comment:
It is not and it is described in details in the above text.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-lang] verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length …
verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName
refactored to create appropriate length …
URL: https://github.com/apache/commons-lang/pull/446#discussion_r325703983
##
File path: src/main/java/org/apache/commons/lang3/ClassUtils.java
##
@@ -433,48 +440,82 @@ public static String getAbbreviatedName(final Class
cls, final int len) {
* "java.lang.String" 5"j.l.String"
* "java.lang.String"15"j.lang.String"
*
"java.lang.String"30"java.lang.String"
+ *
"org.apache.commons.lang3.ClassUtils"18"o.a.c.l.ClassUtils"
*
- * @param className the className to get the abbreviated name for, may be
{@code null}
- * @param len the desired length of the abbreviated name
- * @return the abbreviated name or an empty string
- * @throws IllegalArgumentException if len = 0
+ *
+ * @param className the className to get the abbreviated name for, may be
{@code null}
+ * @param len the desired length of the abbreviated name
+ * @return the abbreviated name or an empty string if the specified
+ * class name is {@code null} or empty string
+ * @throws IllegalArgumentException if {@code len <= 0}
* @since 3.4
*/
public static String getAbbreviatedName(final String className, final int
len) {
- if (len <= 0) {
-throw new IllegalArgumentException("len must be > 0");
- }
- if (className == null) {
-return StringUtils.EMPTY;
- }
-
- int availableSpace = len;
- final int packageLevels = StringUtils.countMatches(className, '.');
- final String[] output = new String[packageLevels + 1];
- int endIndex = className.length() - 1;
- for (int level = packageLevels; level >= 0; level--) {
-final int startIndex = className.lastIndexOf('.', endIndex);
-final String part = className.substring(startIndex + 1, endIndex + 1);
-availableSpace -= part.length();
-if (level > 0) {
- // all elements except top level require an additional char space
- availableSpace--;
+if (len <= 0) {
+throw new IllegalArgumentException("len must be > 0");
}
-if (level == packageLevels) {
- // ClassName is always complete
- output[level] = part;
-} else {
- if (availableSpace > 0) {
-output[level] = part;
- } else {
-// if no space is left still the first char is used
-output[level] = part.substring(0, 1);
- }
+if (className == null) {
+return StringUtils.EMPTY;
}
-endIndex = startIndex - 1;
- }
- return StringUtils.join(output, '.');
+final char[] abbreviated = className.toCharArray();
+int target = 0;
+int source = 0;
+while (source < abbreviated.length) {
+// copy the next part
+int runAheadTarget = target;
+while (source < abbreviated.length && abbreviated[source] != '.') {
+abbreviated[runAheadTarget++] = abbreviated[source++];
+}
+
+++target;
+if (useFull(runAheadTarget, source, abbreviated.length, len)
+|| target > runAheadTarget) {
+target = runAheadTarget;
+}
+
+// copy the '.' unless it was the last part
+if (source < abbreviated.length) {
+abbreviated[target++] = abbreviated[source++];
+}
+}
+return new String(abbreviated, 0, target);
+}
+
+/**
+ * Decides if the part that was just copied to its destination
+ * location in the work array can be kept as it was copied or must be
+ * abbreviated. It must be kept when the part is the last one, which
+ * is the simple name of the class. In this case the {@code source}
+ * index, from where the characters are copied points one position
+ * after the last character, a.k.a. {@code source ==
+ * originalLength}
+ *
+ * If the part is not the last one then it can be kept
+ * unabridged if the number of the characters copied so far plus
+ * the character that are to be copied is less than or equal to the
+ * desired length.
+ *
+ * @param runAheadTarget the target index (where the characters were
+ * copied to) pointing after the last character
+ * copied when the current part was copied
+ * @param source the source index (where the characters were
+ * copied from) pointing after the last
+ * character copied when the current part was
+ * copied
+ * @param originalLength the original length of the class full name,
+ * which is abbreviated
+ * @param desiredLength the desired length of the abbreviated class
+ *
[GitHub] [commons-lang] verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length …
verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName
refactored to create appropriate length …
URL: https://github.com/apache/commons-lang/pull/446#discussion_r317534659
##
File path: src/main/java/org/apache/commons/lang3/ClassUtils.java
##
@@ -987,33 +1005,35 @@ public static boolean isAssignable(Class cls, final
Class toClass, final b
// Inner class
// --
+
/**
* Is the specified class an inner class or static nested class.
*
- * @param cls the class to check, may be null
+ * @param cls the class to check, may be null
* @return {@code true} if the class is an inner or static nested class,
- * false if not or {@code null}
+ * false if not or {@code null}
*/
public static boolean isInnerClass(final Class cls) {
return cls != null && cls.getEnclosingClass() != null;
}
// Class loading
// --
+
/**
* Returns the class represented by {@code className} using the
* {@code classLoader}. This implementation supports the syntaxes
* "{@code java.util.Map.Entry[]}", "{@code java.util.Map$Entry[]}",
* "{@code [Ljava.util.Map.Entry;}", and "{@code [Ljava.util.Map$Entry;}".
*
- * @param classLoader the class loader to use to load the class
- * @param className the class name
+ * @param classLoader the class loader to use to load the class
+ * @param className the class name
* @param initialize whether the class must be initialized
* @return the class represented by {@code className} using the {@code
classLoader}
* @throws ClassNotFoundException if the class is not found
*/
public static Class getClass(
-final ClassLoader classLoader, final String className, final
boolean initialize) throws ClassNotFoundException {
+final ClassLoader classLoader, final String className, final boolean
initialize) throws ClassNotFoundException {
Review comment:
I am doing it. Actually, these formattings are clumsy in the original code,
and IntelliJ just reformatted automatically. I understand that it is no excuse
for creating a messy PR. Sorry.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-lang] verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length …
verhas commented on a change in pull request #446: LANG-1480 getAbbreviatedName refactored to create appropriate length … URL: https://github.com/apache/commons-lang/pull/446#discussion_r317522502 ## File path: src/main/java/org/apache/commons/lang3/ClassUtils.java ## @@ -16,19 +16,11 @@ */ package org.apache.commons.lang3; +import org.apache.commons.lang3.mutable.MutableObject; + import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; - -import org.apache.commons.lang3.mutable.MutableObject; +import java.util.*; Review comment: This was the default IntelliJ setting to use * after more than 4 imports from the same package. I see no reason why not to use '*' in imports these days with such advanced editors, but in case that is the project standard, I comply. Thanks. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
