You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pivot.apache.org by Greg Brown <gk...@me.com> on 2009/11/16 19:37:13 UTC
Re: svn commit: r835842 - in
/incubator/pivot/trunk/core/src/org/apache/pivot/collections: ./ concurrent/
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
> <no...@gmail.com>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 <tv...@gmail.com>
>> 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.
>>>
>>
Re: svn commit: r835842 - in
/incubator/pivot/trunk/core/src/org/apache/pivot/collections: ./ concurrent/
Posted by Greg Brown <gk...@mac.com>.
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
> <no...@gmail.com>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 <tv...@gmail.com>
>> 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.
>>>
>>