Hi Joe,
IIRC name isn’t actually interned with String.intern() but in the VM:s Symbol
table as a name representing a (Java) method. Should be equivalent, as long as
we don’t start comparing it with == with interned Strings. As Andrej wrote,
this is just refactoring of the previous code, to reuse the logic.
cheers
/Joel
On 07 Jun 2014, at 22:34, Andrej Golovnin <andrej.golov...@gmail.com> wrote:
> Hi Joe,
>
>> Sorry for the belated review.
>>
>> Generally the change looks good. One question, in
>>
>> 2803 private boolean matchesNameAndDescriptor(Method m1, Method m2) {
>> 2804 return m1.getReturnType() == m2.getReturnType() &&
>> 2805 m1.getName() == m2.getName() &&
>> 2806 arrayContentsEq(m1.getParameterTypes(),
>> 2807 m2.getParameterTypes());
>> 2808 }
>>
>> Should the equality check on 2805 be .equals rather than == ?
>>
>
> "==" can be used in this case as the method name is interned by JVM.
> Here is the comment for the field "name" from java.lang.reflect.Method:
>
> // This is guaranteed to be interned by the VM in the 1.4
> // reflection implementation
> private String name;
>
> BTW, in the old version of Class in the line 2766 there was already a similar
> check:
>
> 2764 if (m != null &&
> 2765 m.getReturnType() == toRemove.getReturnType() &&
> 2766 m.getName() == toRemove.getName() &&
> 2767 arrayContentsEq(m.getParameterTypes(),
> 2768 toRemove.getParameterTypes())) {
> 2769 methods[i] = null;
>
>
> Best regards,
> Andrej Golovnin
>
>
