There are places in the code that have excessive nesting. For some
reason books show this and everyone does it and I certainly did at
first too, but having to deal with trees of logic 3-4 levels deep can
really be a pain. Many times the logic can be made completely linear
with no else cases. A big sign of that is when you repeat your else
case many times.
I started to work an example in email, but refactoring is hard to
describe once you get beyond one or two steps and decided to just show
you what I mean:
http://people.apache.org/~dblevins/ExcessNesting.mov
That video basically shows how this:
public boolean hasBinding2(List<Class<? extends Annotation>>
bindingTypes, List<Annotation> annots)
{
boolean result = false;
if (bindingTypes != null && annots != null &&
(bindingTypes.size() == annots.size()))
{
int i = 0;
for (Class<? extends Annotation> bindingType :
bindingTypes)
{
if
(this.interceptorBindingSet.containsKey(bindingType))
{
Annotation target =
this.interceptorBindingSet.get(bindingType);
if
(AnnotationUtil.hasAnnotationMember(bindingType, annots.get(i), target))
{
result = true;
}
else
{
return false;
}
}
else
{
return false;
}
i++;
}
}
return result;
}
Can be turned into this:
public boolean hasBinding(List<Class<? extends Annotation>>
bindingTypes, List<Annotation> annots)
{
if (bindingTypes == null || annots == null) return false;
if (bindingTypes.size() != annots.size()) return false;
if (bindingTypes.size() == 0) return false;
int i = 0;
for (Class<? extends Annotation> bindingType : bindingTypes)
{
Annotation target =
this.interceptorBindingSet.get(bindingType);
if (target == null) return false;
if (!AnnotationUtil.hasAnnotationMember(bindingType,
annots.get(i), target)) return false;
i++;
}
return true;
}
Not sure if people like this style of code, but figured I'd share and
see what you all thought.
-David
