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

Reply via email to