FYI.
* I applied Alexey's modified patch. (much appreciated)
* I applied Alexey's new hashCode routine. seems reasonable to me.
* I added NPE checks to equals and hash code. While Alexey's point
these are never needed is a good one, I worry about the consequence of
adding code in the future that uses this class in a new way.
* Similarly, I changed == to equals() in the equals() method. (or
rather, != to !equals)
* Finally, I added a version of Hennings' equals() unit test.
Normally I'd argue a unit test for equals() is overkill. But here,
the equals() method is the main point of MethodCacheKey (and is the
feature that was incorrect in the previous version).
Best, WILL
On 9/28/06, Henning Schmiedehausen <[EMAIL PROTECTED]> wrote:
Hi Alexey,
thanks a lot for your patch. I like it and we should apply it. I very
much appreciate that you understood my mail as peer review and not
critique per se.
[...]
> This is just waste of processor cycles.
> The Class does not override the equals().
Hm. It does not for the Sun JDK and I'm pretty sure that it will not be
overridden by any other JDK, but this is IMHO not guaranteed.
One of my "Java memes" is "thou shalt never compare two objects with ==
or !=". A class object is definitely an object. Yes, the default
implementation of equals() inside Object is '=='.
However, it is the job of the compiler to optimize that away, not of the
developer to second guess it. So for the sake of being anal, I'd like to
see equals() here. :-)
> And it never be overriden.
Sure? /me thinks of weird class loader things and runtime byte code
'improvements'... :-) But again, I'm splitting hairs here...
>
>> mainly because I don't like the implementation of
>> the methods and don't believe in the handwaving that "this is faster
>> than using EqualsBuilder and HashCodeBuilder because of Object
>> creation". I haven't seen any numbers to prove or disprove that claim.
>
> You want to compare the speed of
[... HCB vs. hand-rolled code removed ...]
Yes, because I wouldn't use the Reflection code but add the attributes
explicitly and again, I will not second guess the compiler. I'll try to
get some numbers tonight...
[...]
>> An unit test draft could look like this:
>
> ...
>
>> public void testHashCode()
>> {
>> Class [] e1 = new Class [] { String.class, String.class };
>> Class [] e2 = new Class [] { Object.class, Object.class };
>
>> ASTMethod m1 = new ASTMethod(23);
>> MethodCacheKey mck1 = m1.new MethodCacheKey(e1);
>> MethodCacheKey mck2 = m1.new MethodCacheKey(e2);
>
>> assertTrue(mck1.hashCode() != mck2.hashCode());
>> }
>
> I do not agree with this test. It is not required (and is not always
> possible) that different objects return different hashcodes.
Yes, you are right. That is a leftover and should not be there. As I
said, this was a draft. (I do know that the smallest legal
implementation of hashCode() is public int hashCode() { return 42; } ;-) )
Best regards
Henning
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
--
Forio Business Simulations
Will Glass-Husain
[EMAIL PROTECTED]
www.forio.com
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
