Bob,

Thanks for the reply! Sorry for the confusion - my local copy got mangled and 
told me the files were still there when in fact they had been reverted. 
Deleting my local copy and doing a fresh checkout restored sanity. The bottom 
line is, I took the files back out - so that's why you can't see them. Check 
out the previous revision to see them.

My motivation for committing it is code quality. I like fail-fast designs. If 
you give me a bad argument, then I will give you an exception right back. The 
Assert class has methods that allow you to check arguments in a single line - 
making argument checking very convenient. If the arguments are valid, nothing 
happens. If the arguments are invalid, then an exception is thrown - with a 
message that tells the programmer what's wrong.

If I commit the files, there will be no need to extend Validate - the code is 
simple enough that extending Validate will just make things more complicated. I 
will include one nice feature Validate has that I didn't think of - a list of 
valid choices in the exception message:

"foo is incorrect type. Must be one of MyClass, YourClass, SomeClass."

-Adrian


--- On Wed, 4/14/10, Bob Morley <[email protected]> wrote:

> From: Bob Morley <[email protected]>
> Subject: Re: svn commit: r934179 - in /ofbiz/trunk/framework/base: build.xml 
> src/org/ofbiz/base/util/Assert.java 
> src/org/ofbiz/base/util/test/AssertTests.java
> To: [email protected]
> Date: Wednesday, April 14, 2010, 9:02 PM
> 
> 
> Adrian Crum wrote:
> > 
> >> Comments are welcome.
> > 
> Your first mistake!  heh
> 
> It does not appear my trunk checkout has these classes in
> them ... I am
> guessing that the use here is in standard classes (not in
> junit testers). 
> If you have a second to look at my patch associated with
> OFBIZ-3670; I
> provided a BaseTestCase that provides similar assertions
> that add-on to the
> set provided by junit (I am taking assertEmpty type guys
> that use the
> UtilValidate.isEmpty) ... If the intent was to do this from
> unit testers I
> would consider putting them in this base class.
> 
> As for comments, I would _consider_ the following minor
> tweaks ...
> 
> a) (if possible) create an org.ofbiz.base.util.Validate
> that extends the one
> from Apache commons with your enhancement
> b) would consider overloading "notNull" rather than
> introducing a new method
> c) without seeing the class, my guess is that you have it
> overloaded to
> support n name/value pairs (where n is arbitrarily largish
> ... like 6)  :)
> d) may consider creating a Pair object so we could have a
> signature
> something like ..
> 
> Validate.notNull(Pair.create("foo", foo),
> Pair.create("bar", bar));
> 
> public static void notNull(Pair... objects) {
> ...
> }
> 
> You can also provide a notNull that just takes an
> open-ended set of objects
> and does what the apache commons implementation does (just
> on each one).  Ok
> those are my ramblings ...
> -- 
> View this message in context: 
> http://n4.nabble.com/Re-svn-commit-r934179-in-ofbiz-trunk-framework-base-build-xml-src-org-ofbiz-base-util-Assert-java-sra-tp1840515p1840770.html
> Sent from the OFBiz - Dev mailing list archive at
> Nabble.com.
> 



Reply via email to