Squashed?

So I have created a patch that we have now accepted into the trunk repository.

The issue I worked on was that PackageEqualityMethod was not importing the public/package/protected inner classes to be used in the "areEqual" methods. At first I developed a few new methods into the system to try and grab all instances of inner classes in a specified package but at a code review session we talked about how to take the things I learned from creating those methods and how we can modify the code to grab the inner classes, seeing as how it already grabs everything else.

Here is an example of a class that we are trying to grab:

//Demo of program that creates bug
//would import just fine in PackageEqualityMethods
public class Foo{
    private int x;
 
    public method1(){
        //Do stuff
    }
 
    //would never import into PackageEqualityMethods
    public class Bar{
        private int y;
        private method2(){
              //Do stuff
        }
    }
 
    //never needed to be imported because it is private
    private class PrivClass{
        //Private class
    }
}

This class created 2 areEqual methods, one for Foo and one for Bar. The private method does not generate an areEqual method, but we are considering making that enhancement at a later time if public opinion and want is high enough.

Here is part of the prototype code that was used to grab the inner classes and put them in the import statement list:

//Class TestAbstract
 
//Method to grab all internal classes and add them to the imports
public  void addInnerClasses(Class classToScan) {
        Class[] classes = classToScan.getDeclaredClasses();
        for (Class c : classes) {
            String importString = c.getName();
            //The getName will return the name with a “$” to show it was internal to the class, replace with a “.”
            importString = importString.replaceAll("\\$", ".");
            //Check to see if it is a member or an enum, and if the modifier is not private
            if ((c.isMemberClass() || c.isEnum()) && (!Modifier.isPrivate(c.getModifiers()))) {
                //if the class is not already imported
                if (!dynamicImportContains(importString)) {
                    getDynamicImports().add(importString);
                }
            }
        }
    }

We then found the part of code that imports classes into the PackageEqualityMethod, and modified it to create this:

//code segment in Class TestAbstract
boolean shouldBeIgnored = false;
 
//code to check if it is not already in dynamic imports
 
//make sure: not in same package
if (classToImport.getPackage().toString().compareToIgnoreCase(
  classTested.getPackage().toString()) == 0) {
  //Added statement to allow member/enum/local classes.
  if (!(classToImport.isMemberClass() || classToImport.isEnum()
                     || classToImport.isLocalClass())) {
    shouldBeIgnored = true;
  }
}
 
//rest of the class that creates the import statements

The final solution to fix this bug was a single conditional statement added to make sure that inner classes were not being ignored.

All in all I believe that sitting down with the code and running multiple open source programs through it has brought me to a better understanding of how reflection works in Java, as well as a greater understanding of Obsidian. After submitting the patch (diff file) I have also started looking into what coding standards and patch requirements we might implement when we release the software in 5 weeks, but I'll talk about my findings next time.