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

[Thomas Fletcher <ThomasF@xxxxxxx>]

> -----Original Message-----
> From: cdt-debug-dev-bounces@xxxxxxxxxxx 
> [mailto:cdt-debug-dev-bounces@xxxxxxxxxxx] On Behalf Of Øyvind Harboe
> Sent: June 7, 2005 1:12 PM
> To: cdt-debug-dev@xxxxxxxxxxx; crecoskie@xxxxxx
> Subject: [cdt-debug-dev] So many if() statements in CDT source code
> 
[ Comments snipped ]
> 
> 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);
> 	}

This is handy for specialization.  I haven't looked at the code for this
particular instance but if you had a fast path/high runner for compareTo
that you could invoke with an IQualifiedTypeName then you can do this.
The general Object method can be invoked as well generically so that you
can have code that works 

Allocate a specific instance of IQualifiedTypeName (MyQualifiedTypeName)
-- Pass through an IQualifiedTypeName interface barrier
Come back through as something more generic but on which you can
invoke compareTo() methods such as an ITypeName 
 
> 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;
> 	}

You have to be carefull, just because nothing in CDT uses it, doesn't
mean that someone else isn't using is as part of a framework.  Also
in this case it could be that an ICElement can be created from the
IFile resource via some factory method, but that the ICElement is not 
identified as a binary file (classification done by the binary parsers 
I believe).
 
> 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

No comment on this one.  Not a fan of thingy's myself =;-)

Thomas