geronimo-xbean-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Blevins <david.blev...@visi.com>
Subject Re: svn commit: r731945 - in /geronimo/xbean/trunk/xbean-finder/src: main/java/org/apache/xbean/finder/ClassFinder.java test/java/org/acme/foo/FunnyFamilyHalloween.java test/java/org/apache/xbean/finder/ClassFinderTest.java
Date Fri, 09 Jan 2009 00:47:28 GMT

On Jan 6, 2009, at 6:19 AM, jlaskowski@apache.org wrote:

> Author: jlaskowski
> Date: Tue Jan  6 06:19:28 2009
> New Revision: 731945
[...]
>
> +            for (int pos = 0; pos < tempClassInfos.size(); pos++) {
> +                ClassInfo classInfo = tempClassInfos.get(pos);
> +                try {
> +                    String superType = classInfo.getSuperType();
> +                    for (Class clazz : classes) {
> +                        if (superType.equals(clazz.getName())) {
> +                            classes.add(classInfo.get());
> +                            tempClassInfos.remove(pos);
> +                            annClassFound = true;
> +                            break;
> +                        }
> +                    }
> +                } catch (ClassNotFoundException e) {
> +                    classesNotLoaded.add(classInfo.getName());
> +                } catch (NoClassDefFoundError e) {
> +                    classesNotLoaded.add(classInfo.getName());
> +                }
> +            }

We should change up this algorithm to either not remove while  
iterating (i.e. maybe instead add to another list that starts out  
empty) or add a "pos--;"  after the remove statement.  Basically, when  
you remove an item the next item becomes the current value of  
position, but the loop's next iteration goes to position +1 resulting  
in the next item being skipped on every remove.  Here's an example:

public class FooTest extends TestCase {
    public void test() throws Exception {
        List<Integer> list = new  
ArrayList(Arrays.asList(1,2,3,4,5,6,7,8,9));
        for (int i = 0; i < list.size(); i++) {
            list.remove(i);
        }

        for (Integer item : list) {
            System.out.println(item);
        }
    }
}

The result of this is that all the even numbers will still be there.

Looking at the code though, I bet if we changed classInfos from a list  
to a map it would make that whole algorithm a little easier to write.   
Optionally, we could even add a "List<ClassInfo> subclasses" and  
"ClassInfo superclass" to the ClassInfo and link them together in the  
related ClassFinder constructors.

It's great to have you in the code!

-David


Mime
View raw message