[jira] [Work logged] (LANG-1480) ClassUtils. getAbbreviatedName(String ,int) returns too long result

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[jira] [Work logged] (LANG-1480) ClassUtils. getAbbreviatedName(String ,int) returns too long result

ASF GitHub Bot (Jira)

     [ https://issues.apache.org/jira/browse/LANG-1480?focusedWorklogId=314347&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-314347 ]

ASF GitHub Bot logged work on LANG-1480:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 18/Sep/19 14:20
            Start Date: 18/Sep/19 14:20
    Worklog Time Spent: 10m
      Work Description: verhas commented on 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) {
      * <tr><td>"java.lang.String"</td><td> 5</td><td>"j.l.String"</td></tr>
      * <tr><td>"java.lang.String"</td><td>15</td><td>"j.lang.String"</td></tr>
      * <tr><td>"java.lang.String"</td><td>30</td><td>"java.lang.String"</td></tr>
+     * <tr><td>"org.apache.commons.lang3.ClassUtils"</td><td>18</td><td>"o.a.c.l.ClassUtils"</td></tr>
      * </table>
-     * @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 &lt;= 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);
+    }
+
+    /**
+     * <p>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}</p>
+     *
+     * <p>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.</p>
+     *
+     * @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
+     *                       name
+     * @return {@code true} if it can be kept in its original length
+     * {@code false} if the current part has to be abbreviated and
+     */
+    private static boolean useFull(final int runAheadTarget,
 
 Review comment:
   Is this a strong requirement or something you can live with if I leave it as it is?
   
   As you can see from the length of the comment this is a complex expression and the JavaDoc explains a lot about the algorithm used. Such an extensive commend would degrade the readability of the caller method.
   
   As per the performance: the JIT will inline this method, it should not be a problem.
 
----------------------------------------------------------------
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:
[hidden email]


Issue Time Tracking
-------------------

            Worklog Id:     (was: 314347)
    Remaining Estimate: 20h 10m  (was: 20h 20m)
            Time Spent: 3h 50m  (was: 3h 40m)

> ClassUtils. getAbbreviatedName(String ,int) returns too long result
> -------------------------------------------------------------------
>
>                 Key: LANG-1480
>                 URL: https://issues.apache.org/jira/browse/LANG-1480
>             Project: Commons Lang
>          Issue Type: Bug
>          Components: lang.*
>    Affects Versions: 3.9
>         Environment: Environment independent.
>            Reporter: Peter Verhas
>            Assignee: Bruno P. Kinoshita
>            Priority: Major
>   Original Estimate: 24h
>          Time Spent: 3h 50m
>  Remaining Estimate: 20h 10m
>
> In some cases, the algorithm decides incorrectly when to which package names to abbreviate. For example, abbreviating
> {{org.apache.commons.lang3.ClassUtils}} to the length 18 will result {{o.a.c.lang3.ClassUtils}} (22 characters) instead of {{o.a.c.l.ClassUtils}} (18 characters as requested). The reason for this is that the algorithm starts from the right and goes to the left abbreviating the packages and starts to abbreviate the packages when it runs out of the available space.
> Instead, the algorithm should start from the left and abbreviate all packages that would result in a too-long string without abbreviating the package name.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)