Great. Good to know where everyone's head is at style wise.
I made the described change. Broke the single line ifs into multi
line braced style as Mark suggested -- both styles are used in the
code now, but I don't have a problem throwing them in.
I like single line simple ifs for returns, but wish java allowed you
to put them at the left side of the statement where they are more
visible -- something I like about Perl.
public boolean hasBinding(List<Class<? extends Annotation>>
bindingTypes, List<Annotation> annots)
{
return false if (bindingTypes == null || annots == null);
return false if (bindingTypes.size() != annots.size());
return false if (bindingTypes.size() == 0);
int i = 0;
for (Class<? extends Annotation> bindingType : bindingTypes)
{
Annotation target =
this.interceptorBindingSet.get(bindingType);
return false if (target == null);
return false if (!
AnnotationUtil.hasAnnotationMember(bindingType, annots.get(i), target));
i++;
}
return true;
}
Works out sort of like a ternary but without the else case, which is
even shorter. A guy can dream. Maybe we'll see it in Java 11 (or
some far off version).
-David
On Sep 30, 2009, at 3:17 AM, Gurkan Erdogdu wrote:
Hi David;
Actually both styles (brace like codes and fail-early like codes)
exist in
the current implementation.
IMO, parameter/-error condition- early checking at the start of
method is
always a good practice. For example, we use "Asserts.notNull()" like
methods
in several utility methods.
--Gurkan
2009/9/30 David Blevins <[email protected]>
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<http://people.apache.org/%7Edblevins/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
--
Gurkan Erdogdu
http://gurkanerdogdu.blogspot.com
