[jbossseam-issues] [JBoss JIRA] Updated: (JBSEAM-3745) org.jboss.seam.util.Strings improvements

[Anthony Whitford (JIRA)]

     [ https://jira.jboss.org/jira/browse/JBSEAM-3745?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Anthony Whitford updated JBSEAM-3745:
-------------------------------------

    Attachment: JBSEAM-3745.patch


Attached is a patch to address the issues outlined.

> org.jboss.seam.util.Strings improvements
> ----------------------------------------
>
>                 Key: JBSEAM-3745
>                 URL: https://jira.jboss.org/jira/browse/JBSEAM-3745
>             Project: Seam
>          Issue Type: Patch
>          Components: Core
>    Affects Versions: 2.1.1.CR1
>            Reporter: Anthony Whitford
>         Attachments: JBSEAM-3745.patch
>
>
> I noticed some improvements that could be made to the core Strings utility class.
> 1.  The isEmpty method is inefficient:
>    public static boolean isEmpty(String string)
>    {
>       return string == null || string.trim().length() == 0; 
>    }
> See http://pmd.sourceforge.net/rules/strings.html#InefficientEmptyStringCheck
> I think you really want something closer to Apache Commons Lang StringUtils.isBlank:
>  - http://commons.apache.org/lang/api-release/org/apache/commons/lang/StringUtils.html#isBlank(java.lang.String)
>  - http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/java/org/apache/commons/lang/StringUtils.java?view=markup
> 2.  Consider the code:
>    public static String toString(String sep, Object... objects)
>    {
>       if (objects.length==0) return "";
>       StringBuilder builder = new StringBuilder();
>       for (Object object: objects)
>       {
>          builder.append(sep).append(object);
>       }
>       return builder.substring(2);
>    }
> The magic number 2 passed to the substring looks suspicious.  I think this assumes that sep has a length of 2.  This utility should avoid making assumptions.  A similar function avoids the assumption by using sep.length:
>    public static String toString(String sep, Class... classes)
>    {
>       if (classes.length==0) return "";
>       StringBuilder builder = new StringBuilder();
>       for (Class clazz: classes)
>       {
>          builder.append(sep).append( clazz.getName() );
>       }
>       return builder.substring(sep.length());
>    }
> Overall, the hard-coded 2's should be replaced with sep.length.
> 3.  StringBuilder is used, but some StringBuffer remains -- those could be replaced.
> 4.  Some appends on one character strings could be replaced with char versions.  (See http://pmd.sourceforge.net/rules/strings.html#AppendCharacterWithChar ).

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira