Agreed.
On Nov 16, 2009, at 1:32 PM, Todd Volkert wrote:
Actually, I think it's probably best to remove the class and move
verifyNotNull() into the constituent classes inline. We have a ton
of null
arg checks elsewhere in the framework that don't use a dedicated arg
check
class, so it feels weird to introduce one here. I'm open to
discussing the
existence of such classes (though I personally tend to shy away from
them),
but using it in this one package seems inconsistent and out of place.
-T
On Sat, Nov 14, 2009 at 12:47 AM, Noel Grandin
<[email protected]>wrote:
I've renamed the methods and moved most of them into the caller
classes.
It seems a shame to move verifyNotNull(), though, because it is used
in so many places ... ?
I can't make it package-private because it is used in
collections..concurrent
On Fri, Nov 13, 2009 at 20:11, Todd Volkert <[email protected]>
wrote:
- I would prefer to see the methods named "validateXXX()" or
"verifyXXX()".
+1 - I like these names better.
- I would prefer to see them defined within the classes that call
them,
rather than delegating to a separate CollectionArgChecks class.
Even if
it
introduces a bit of redundancy, I think the encapsulation offered
by
this
approach is preferable.
I personally don't actually mind the separate class in this case,
but I
think it should be package private.
