Thanks a lot Henri! I talked with Shawn Pearce about possible performance issues and we now ended up with 3 function groups:
notNull(Object) notNull(Object, String) notNull(Object, String, Object...); So the JVM doesn't need to create an Object[] if we only like to provide a single String message. I think we could do the same for all the notNull(Object, String, Integer) for binary compatibility although I believe this will make the library more complex to use. And since the packages will change too ... In any case we should do 2 things (also for the 'old' functions with (Object, String, Integer) or whatever). Remember ALL those things will happen _only_ if the validation fails, so we don't have to care about performance - we don't need to fail fast ;) 1.) if (o == null ) { if (msg contains {0} or something that indicates the use of MessageFormat) { use MessageFormat; } else { use msg + ": " + msgIntParam; } } So this should then even be compatible to use for 'old' invocations where one doesn't like to use the MessageFormat feature. wdyt? Patch will follow in about ~1-2 weeks. txs and LieGrue, strub --- Henri Yandell <flame...@gmail.com> schrieb: > trunk/ is now JDK 5 focused. It'll be Lang 3.0, but is very unlikely > to keep the same package naming. > > My biggest concern with ellipsis code is that null moves from being > null to being null inside an Object[]. So we could see some > unnecessary API changing if we're not careful. > > Otherwise - the below sounds good and a JIRA issue with a patch would rock :) > > Thanks, > > Hen > > On Sat, May 2, 2009 at 10:06 AM, Mark Struberg <strub...@yahoo.de> wrote: > > > > Hi! > > > > I'm a long time user (and big fan) of commons.lang.Validate because it's a > > very neat pattern > for getting stable software modules. > > > > I'm PMC member on Apache OpenWebBeans and currently also writing the > > maven-scm-provider-jgit > (JGIT is a native Java implementation of GIT). Since jgit-core (the part of > EGIT we use for the > maven-scm-provider-jgit) is BSD and Shawn likes to have not too much external > dependencies, I > started writing my own little Validate helper class and had a hopefully good > idea which imho > would also be a good extension to commons.lang.Validate: > > > > A main problem on validation is the message creation which costs a lot of > > String operations > and therefor also garbage collection. I've now seen that the latest version > in SVN has an > additional object parameter for a few functions which addresses this problem. > My proposal now > goes even further but requires java-5 since it uses ellipsis notation. > > > > If msgObject[0] is a String java.text.MessageFormat will be used for > > creating the failure > message, e.g. > > Validate.notNull(nullInt, "testMessageText Integer={0} btw! and > > Integer2={1}.", new > Integer(42), new Integer(43)); > > > > example for isTrue with message construction: > > > > /** > > * Validate that b is <code>true</code> > > * > > * @param b boolean to validate > > * @param msgObjects additional Objects added as text message to the > > InvalidArgumentException > > */ > > public static void isTrue(boolean b, Object... msgObjects) { > > if (!b) { > > throw new IllegalArgumentException(getMessage(msgObjects)); > > } > > } > > > > /** > > * private helper function to create an error message from the given Objects > > * If the first object in msgObjects is of type {...@code String} then > > * {...@code MessageFormat} will be used to format the output message. > > * > > * @param msgObjects > > * @return concatenated String representation of all the objects > > */ > > private static String getMessage(Object... msgObjects) { > > if (msgObjects.length > 0 && msgObjects[0] instanceof String) { > > MessageFormat form = new MessageFormat((String) > > msgObjects[0]); > > Object[] params = new Object[msgObjects.length - 1]; > > System.arraycopy(msgObjects, 1, params, 0, msgObjects.length > > - 1); > > return form.format(params); > > } > > else { > > StringBuffer sb = new StringBuffer("Validation failed: ["); > > for(int i = 0; i < msgObjects.length; i++) { > > if (i > 0) { > > sb.append(' '); > > } > > sb.append(msgObjects[i]); > > } > > sb.append(']'); > > return sb.toString(); > > } > > } > > > > I've tested those functions against 'old' Validate handling to ensure that > > there are no > performance side effects with Java ellipsis handling, and the performance win > is huge [1]. The > ellipsis Version only takes < 5% of the time of the 'old' handling. > > > > WDYT? > > a.) Is the implementation ok? > > b.) Will there be a java-5 only version of commons.lang in the near future? > > c.) Imho we could replace the 1-parameter versions with the proper ellipsis > > functions and we > are still compile compatible. But I'm not sure if we stay binary-compatible > (needs to be > tested). > > > > I wrote a Validate class this way for jgit-core [2] but also have to like > > this features in our > owns commons.lang.Validate if possible! > > > > > > txs and LieGrue, > > strub > > > > [1] http://pastebin.com/m2cf887a9 > > > > [2] > http://github.com/sonatype/JGit/blob/2ab3a576fa67145d6a9f66efd7437c52d567eb68/org.spearce.jgit/src/org/spearce/jgit/util/Validate.java > > > > > > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org