Thursday, September 15, 2005

Posted on the bug (Eclipse Bugzilla Bug 108071)

I have studied the code in NewTypeWizardPage, CodeGeneration, StubUtility, and the 2 other references to the CodeGeneration.getCompilationUnitContent API in the jdt package (in AccessorClassCreator and MoveInnerToTopRefactoring).

The bad news is I can't see any way to do this enhancement without changing the getCompilationUnitContent API.

The biggest reason is that StubUtility.getCompilationUnitContent is responsible for evaluating templates and resolving the template variables. With this change, I will have to shift the responsibility for inserting the content of a new type and its stub methods from their current hard-coded treatment in NewTypeWizardPage down into StubUtility, so I will have to pass more information (for new template variables) into StubUtility.getCompilationUnitContent to allow it to resolve all the template variables at once.

A secondary reason is that the client interaction with StubUtility.getCompilationUnitContent is currently tightly coupled with its clients. All clients currently are calling getLineDelimiterUsed from StubUtility directly, and usually making separate calls to getTypeComment and getFileComment and passing the results back to the call to CodeGeneration.getCompilationUnitContent. In addition, two of the clients have essentially repeated fail-safe code after the call to getCompilationUnitContent returns. (These are NewTypeWizard.constructCUContent and MoveInnerToTopRefactoring.createSourceForNewCu.) Basically, the interaction between the clients and the getCompilationUnitContent method could be made a lot cleaner and cries out to be refactored.

I doubt if it is even possible to keep the current version of the API around as a facade that calls the new routine in the background, because I predict I will run into some unreconcilable failure cases and strange user experience when the user uses new templates and the old code doesn't allow the data to be passed in to fill them. The same goes for creating new parallel code and leaving the old code intact, which has the added problem of creating a duel maintenance problem. I would much rather clean up the current code for all concerned and for any future enhancements as well.

So, what I'm willing to do to fix all of this is:
Add additional tests to cover the new functionality and combinations, alter any existing tests as needed to pass with the new API.
Fix the getCompilationUnitContent API to be more flexible and to require less coupling between its clients and itself.
Refactor the current clients to use the new API and clean up repeated code made unnecessary by the change.

As far as the exact form the new API will take, let me prototype a few possibilities, and then come back with a suggestion or some choices.
So let me know what you think. I know this isn't what you wanted to hear, but if you approve me doing this, I have a way forward and can get this done without too much trouble. (In other words, it's a tractable/straighforward problem as long as you accept the constraint that the API has to change.)

MoveInnerToTopRefactoring.createSourceForNewCu(ICompilationUnit, IProgressMonitor)


Post a Comment

<< Home