[prev in list] [next in list] [prev in thread] [next in thread] 

List:       velocity-dev
Subject:    Re: svn commit: r449347 - in /jakarta/velocity/engine/trunk:
From:       Henning Schmiedehausen <hps () intermeta ! de>
Date:       2006-09-28 8:59:06
Message-ID: 451B8EDA.8000600 () intermeta ! de
[Download RAW message or body]

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: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org

[prev in list] [next in list] [prev in thread] [next in thread]