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

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 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/String... - http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/java/org/apach... 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 ).