A couple of replys to my last bugzilla comment came after the weekend. The first one is the first I've heard from Martin, whom Dirk introduced in his first reply as the code template expert.

- - - - - - - - - -

Hi John,
I would favor a new code template 'type body' that will be used for the variable
${type_declaration} in the new file template (I don't see why a new variable
would have to be introduced, Dirk?).
CodeGeneration.getCompilationUnitContent can stay the same. The type content
that is passed to it should be generated using the new type body template. A new
API getTypeBodyContent() would be added for that, and it should get all
information that it requires (type delaration header, type name, cu name...)
When creating a new type, the type wizard would first construct the new cu using
these templates and on that insert the all methods to be created. I don't see
how you could solve that with variables for all created methods (e.g.
${main_method}}. We can't know what kind of methods will be created by clients.
One problem is that all client of
CodeGeneration.getCompilationUnitContent will have to be updated, there will
always be clients that aren't.
An other probelm is that i'm not sure if you really will get what you want. New
methods will probably be entered at the very beginning, and can't understand
your comments in the body. This will be also the case for all other new members
that will be added programatically.

------- Additional Comment #7 From Matt Sanford 2005-09-19 10:18
[reply] ------- The discussed changes will also fix the common template complaint of wanting
to add default members for logging and versioning.

- - - - - - - - - -
My answer to Martin:

Martin,

Thanks for your ideas. Here are my responses.

I don't want to put words in Dirk's mouth, so jump Dirk in if I get this wrong, but I think he wanted me to be conservative about changing the current behavior of the ${type_declaration} variable so it wouldn't break any current tests or invalidate any current client uses.

I was planning to default the new template so it implemented the current hard-coded behavior out of the box, and so would pass all the tests. Since I haven't yet proven I can do that successfully, I can understand Dirk wanting to be cautious about it. I have always hoped I could prove it though, so I don't have to enter a new template variable, because it would mean two template variables that do the same thing, one hard-coded, one user-configurable, and I believe that would be confusing to users.

I had first envisioned, as you imply, that eclipse would generate all new methods at the top of the class, and users would have to cut-and-paste them down into their class where they wanted them to go. If you look at Dirk's first response, however, you can see he suggests creating template variables that direct eclipse to place generated methods at user-specified points within the class. So, you could have methods corresponding to the different options in the new class dialog go different places in the newly generated class. For example:

${type_header} {
// constants

// instance variables

// constructors
${superclass_constructors}

// inherited methods
${inherited_abstract_methods)

// main method
${main_method}
}

I agree with you this isn't a perfect solution, as we can't anticipate all the kinds of methods different clients can generate, and hard-code template variables for them. However, I agree with Dirk this is a better user experience than putting all the generated methods at the top of the class, and we could provide template variables for all the generated methods we currently know about, if we have a version of a template evaluating method that takes a collection of structures that include template variable name, optional content for the name and some flags to indicate how to process (supress as in filecomment if user deselects "Generate comments", supercede passed content with user template variable if present, or always use passed content). (I suggest this as the workable alternative to hard-coding each template variable as a parameter in the signature of the call that evaluates the template. It could be a new version of getCompilationUnitContent if you want to evaluate the whole template at once, or a routine that would replace the guts of NewTypeWizardPage.constructTypeStub if that's what you guys want me to do. Speaking of which...)

The last point to discuss is changing the CodeGeneration.getCompilationUnitContent signature. I see your point that I could accomplish this by pre-evaluating the templates to generate the typeContent, (however complex that becomes with these added method generation variables or not), then passing the resulting typeContent into the current API. I will do it that way (or at least attempt it) if you really want me to. But I think that would be perpetuating (and making worse) the bad coupling and weak cohesion that already exist between and among NewTypeWizardPage and StubUtility/CodeGeneration.

I would go back to the arguments I made in my last comment. And I'll add my opinion that StubUtility should have the responsibility of evaluating templates and reading variables and the NewTypeWizardPage should simply direct StubUtility as to what to do for particular cases. I think this would be a big improvement over the current state, where clients have to make several calls to StubUtility to gather data, pass the data back as parameters to StubUtility in getCompilationUnitContent, and then check for success and fix up a hard-coded default in case of failure. Every client currently requires a paragraph-length interaction with StubUtility to make a getCompilationUnitContent call, and there is a lot of repeated code between clients. I would like to try to reduce that to a few lines.

If it were my code base, that's what I would do. I think it fits the spirit of open-source, where we try to constantly refactor and improve the software to help it reach it's best design. On the other hand, I'm not a committer, and I'm not so totally wedded to this idea (i.e. changing the getCompilationUnitContent API) that I won't do it the other way (keeping it the same) if you guys insist.

Let me know about changing the API and about the new template variables (${superclass_constructors}, ${inherited_abstract_methods}, and ${main_method}) and I'll forge ahead with whatever you think is best.

Thanks,
- John