Re: [Cdk-devel] [ cdk-Patches-2977074 ] new IChemObjectBuilder API

[Mark R. <ma...@eb...>]

On 03/30/2010 02:25 PM, Egon Willighagen wrote:
> On Tue, Mar 30, 2010 at 3:13 PM, Mark Rijnbeek<ma...@eb...>  wrote:
>    
>> Perhaps a better way to sort the list is the CPU Shallow time being
>> according to JDeveloper 'Cumulative Time Spent in This Method Only'.
>> See new screen shot. The winner for that is
>> java.util.AbstractList.Iterator(), with the main contributor being
>> notifyChanged() calling it, iterating over its listeners. I think that's how
>> it works.
>>      
> Can you please have a look at the patch I proposed on Sunday, in my
> email to cdk-devel title "RFC: factoring out the IChemObject listener
> framework"?
>
> Egon
>
>
>    

Hi Egon,

I had a first look at the patch you proposed.

A thing I noticed is that the code now has many lines like this:
             if (atoms[f] instanceof IChemObjectChangeNotifier) {
                 ((IChemObjectChangeNotifier)atoms[f]).addListener(this);
             }
It doesn't look too elegant a solution, it's a bit repetitive (counted 
already 35 times "instanceof IChemObjectChangeNotifier" in the patch).
Moreover when I used "instanceof" in a loop at one point in Orchem, I 
noticed performance degrading quickly. Things ran definitely faster 
after I'd fixed that, although when now Googling for "java performance 
instanceof" most people seem to be of the opinion that modern JVMs 
should be smart enough to deal with this.

What struck me however was with regards to "instanceof" you do get a lot 
of Java purists reacting like on
http://stackoverflow.com/questions/103564/the-performance-impact-of-using-instanceof-in-java
http://www.coderanch.com/t/202667/Performance/java/Cost-call-instanceof#982543

claiming "instanceof" points to a bad code OO design, and suggesting 
alternative solutions like:
"This sounds like a classic implementation of the "Command pattern." To 
remove the instanceofs, you just give the base command class an abstract 
method, and move all the class-specific code into the subclass's 
implementations of that method. Then in the server, you simply call the 
single method on the incoming command; all the type-specificity is then 
handled by polymorphism rather than by if-instanceof-then."


I thought this might be interesting food for thought. Don't shoot the 
messenger!

Mark