The Awful Project File Subsystem of DrJava Will Be Gone!

Hurray, we’re finally getting rid of the awfully over-engineered, needlessly complex, brittle and confusing project file subsystem of DrJava. We’ll keep the files in the current state and retain the ability to read old project files, but we’ll never ever touch them again, and we will always save in the new format.

The subsystem is too big to completely show here on the Code Pranger, but I found instructions on what must be done to add a property to the project file:

/** This parser uses the s-expression parser defined
* in the util pacakge. The SExp tree given by the parser is
* interpreted into a ProjectFileIR that is given to the user.
* This class must also deal with different versions of the project file.
*
* If at some point new information is to be stored in the project
* file, the following places in the code that need to changed:
* -- If the new information pertains to a document, the
* DocFile class should be augmented to store the new info.
* -- The interface for the DocumentInfoGetter should be
* expanded to allow for the new data to be retrieved.
* -- Add a new clause to the else-if ladder in the FilePropertyVisitor.
* -- Add the new information to the DocFile from the
* DocumentInfoGetter in the ProjectFileBuilder's
* addSourceDocument method.
* -- If the change is at the top level, you must modify the
* evaluateExpression method in this parser and add the
* corresponding methods to the ProjectFileIR, ProjectFileIRImpl,
* and ProjectFileBuilder.
*/

So let’s see: If you are only adding some more data that describes a document, you just have to change three classes, and one method. That’s not true at all. You have to change the OpenDefinitionsDocument class, the inner classes ConcreteOpenDefDoc in both AbstractGlobalModel and DefaultGlobalModel, and perhaps one or more of the interfaces it implements as well, for instance INavigatorItem.

And if the data you are storing is of some new form, in a format for which we have no visitors (sub-parsers), then you have to write your own visitors. For example, for a list of breakpoints, I had to write this very short snippet, just to parse (@Please don’t criticize the coding style. I wrote this code, but I mimicked the style the project file subsystem was written in.@):

/** Parses out a list of breakpoint nodes. */
private class BreakpointListVisitor implements SEListVisitor< List> {
public List forEmpty(Empty e) {
return new ArrayList(); }
public List forCons(Cons c) {
List list = c.getRest().accept(this);
DebugBreakpointData tmp = ProjectFileParser.ONLY
.parseBreakpoint(c.getFirst(), _srcFileBase);
list.add(0, tmp); // add to the end
return list;
}
};

public DebugBreakpointData forEmpty(Empty c) {
if ((fname == null) || (offset == null) || (lineNumber == null)) {
throw new PrivateProjectException("Breakpoint information "+
"incomplete, need name, offset and line tags");
}
if (pathRoot == null || new File(fname).isAbsolute()) {
final File f = new File(fname);
return new DebugBreakpointData() {
public File getFile() { return f; }
public int getOffset() { return offset; }
public int getLineNumber() { return lineNumber; }
public boolean isEnabled() { return isEnabled; }
};
}
else {
final File f = new File(pathRoot, fname);
return new DebugBreakpointData() {
public File getFile() { return f; }
public int getOffset() { return offset; }
public int getLineNumber() { return lineNumber; }
public boolean isEnabled() { return isEnabled; }
};
}
}
}

And when the data that you want to add does not pertain to one particular document, but is rather a property of the entire project, then you need to change ProjectFileIR, ProjectFileIRImpl and ProjectFileBuilder. At least that’s what the description says. But you’ll also have to change the AbstractGlobalModel, its subclass DefaultGlobalModel, perhaps one or more of its superclasses or implemented interfaces, and definitely also the inner class ProjectFileGroupingState in AbstractGlobalModel. And since we want to be able to treat both project mode and flat-file mode the same, you then modify the interface ProjectFileGroupingState is implementing, and that forces you to change the inner class FlatFileGroupingState in AbstractGlobalModel as well. Oh, and of course you may have the parsing problem again and have to write that lil’ bit of code up there again.

Once you’ve done all that, you finally get to the easy part of adding GUI components to the “Project Properties” dialog. I hate GUI work, but after the ordeal described above, it felt like a vacation.

I’m the only developer that has added (@One developer changed the format slightly by changing the name of a property.@) anything to the project file format during the last three years and eight months. I’m the only one who had to go through this pain since July 2004. In fact, I have suffered more than the person who initially created this complex construct, because after it was in place, only three small pieces of data were added, and then the project file format remained unchanged for a year and eight months, until I came along.

I have since added persistent breakpoints and watches from the debugger to the project file, persistent bookmarks and information about where the project’s jar file should be created, and using which options. I can’t exactly tell who came up with this code that has tortured me for hours because the SourceForge repository history doesn’t reach far enough back, and I wouldn’t mention the name here anyway, but it must have been written by someone who completely misunderstood fundamental principles of object-oriented design. The code is generally rather well written, assuming that nothing changes: It is correct and robust. However, the developer paid no attention at all to extensibility and flexibility, and made it a nightmare to extend the system.

This is a clear example of over-engineering, of thinking of doing it “the right way”, of creating a large object-oriented design that incorporates nearly all design patterns the developer partially understands. This is an example of code that made the original developer feel good about himself, because he came up with something he considered “pretty”, even though something much simpler and much less pretty would have sufficed, and in the long term prevented a lot of silent anger and premature aging.

Of course my XMLConfig class has about 600 lines of code and 500 lines of unit tests, and it relies on Java’s classes for dealing with XML files, but there’s no reason to reinvent the wheel. In fact, writing an S-expression parser for the project files is another mark of a bad developer who overestimates himself: Instead of looking around and using code that already exists and that has been tested, the macho programmer decides he can do better and without any reason writes the code himself all over again. And because this programmer isn’t as good as he thinks he is, and because he is human, he makes mistakes. The repository history shows several commits that had to fix bugs in the project file code.

XMLConfig is actually very simple code. It just takes a string, which describes a path, follows that path into the AST defined by the XML file, and then returns the value found at the end of the path. And once I got past writing and testing XMLConfig, which did not take long, I have been using it in almost every project that needs to save data. Instead of requiring code like the 80-line extract above, my code using XMLConfig can load a list of file names in a single line.

Soon, two of our current DrJava developers taking the COMP 312 “Production Programming” course will add a complex data structure to the project file, and I don’t want them to go through what I had to go through. I don’t want them to write something like the 80 lines above. And I want to get to a point where adding information to the project file is really easy, because right now the current project file subsystem acts as a huge deterrent against any kind of change.