On 07/15/2013 04:36 PM, Peter Levart wrote:
> I was guided by the Collection.contains() signature:
> 
> Collection<T> {
>     boolean contains(Object o);

Dunno. This was done for compatibility with non-generic code. In your
code, you seem always know the type of the argument, it does not bother
me to check to have the "T element" in parameter.


>>   - Also, you might probably put the null-check in contains(), and
>> simplify the usage
>         if (!contains(selectAnnotationClasses, annotationClass)) {
> 
> I would immediately jump up with questions: What about when
> selectAnnotationClasses is null? Will there be NPE, false or true
> answer? Let's check what contains() does...

Set-theoretically speaking, I would expect it returning false for either
"empty" or "null" array. But then again, I can relate to the need to
cross-check that in contains() once I spot this in the code.

>>   - parseAnnotation2 seems a bad name; it seems just overloading
>> parseAnnotation is good.
> 
> Even here I followed the prior art. 

Sounds fair.

> So should we change '2' to some prefix/suffix word then?

I always thought the numberry suffixes are there to segregate the vastly
different implementations. E.g. multiple methods the pure Java method
calling the native method with doing the it's bidding. Or, multiple
public methods delegating to the same internal one, reshuffling the
arguments. I think the "0" suffix is the good style to follow in these
cases.

-Aleksey.

Reply via email to