[cdt-debug-dev] So many if() statements in CDT source code

[Øyvind Harboe <oyvind.harboe@xxxxxxxxx>]

> I count 5472 "instanceof" in CDT. Note that "instanceof" is just one
> incarnation of "IF a is of type THEN". It can take other forms.

Another form of "IF a is of type THEN" is "if (a != null)". 

> A lot of these are required due to the way event notifications are patterned in the platform.
> Event notifications give an Object as the source of the event, and you have to check the type
> of the source to see if it's an event you care about.

I'm not willing to give up on polymorphism in this case, but lets see if
some specific examples can illuminate things.

> As I said before, show us something specific you feel should be refactored and
> we can have a more meaningful discussion.  I can't say much more than
> many of these are entirely valid for the above reason.  It's entirely 
>possible that there are some which aren't, but I'm sorry to say that
>with 3.0 deadlines looming close, the committers likely do not have the
> time to spend looking for a needle in a haystack.

I don't expect anything for 3.0. If some progress could be made towards
making new code easier to write, that would already be something.

Picking some random examples from a search for instanceof:

1. Hmmm... isn't this instanceof just plain superfluous?

	public int compareTo(Object obj) {
		if (obj == this) {
			return 0;
		}
		if (!(obj instanceof IQualifiedTypeName)) {
			throw new ClassCastException();
		}
		return compareTo((IQualifiedTypeName)obj);
	}

2. CoreModel.java. Hmmm... I'm not entirely sure that this method is
   invoked anywhere. Search showed up with 0 references.

	/**
	 * Return true if IFile is a shared library, i.e. libxx.so
	 */
	public boolean isSharedLib(IFile file) {
		ICElement celement = create(file);
		if (celement instanceof IBinary) {
			return ((IBinary)celement).isSharedLib();
		}
		return false;
	}

3. BreakpointManager.java. This code is tricky to understand, but
   I'll make a fleeting vain attempt at polymorphism. 
   Below I've replaced the  instanceof checks with a new method
   "setThingy()".
  
	breakpoint.setCondition0(newCondition);
	if (breakpoint instanceof LocationBreakpoint) {
		setLocationBreakpoint((LocationBreakpoint)breakpoint);
	} else if (breakpoint instanceof Watchpoint) {
		setWatchpoint((Watchpoint)breakpoint);
	} else {

... => rewrite to ....

	breakpoint.setCondition0(newCondition);
	breakpoint.setThingy(this);  // polymorphism
	





> Better yet would be if you send in a patch :-)

In my experience it is best to have some sort of common understanding of
a  mutually agreeable solution before attempting a patch.


-- 
Øyvind Harboe
http://www.zylin.com