Is it generally preferred to use JavaTemplate instead of new'ing up elements "manually"?

[aegershman]

(Questions at the bottom) Occasionally I'll find myself new'ing up items when I need to insert them, like inserting a J.Modifier by doing new J.Modifier(randomId(), Space.EMPTY, Markers.EMPTY, J.Modifier.Type.{Private,Public,etc.}.

For example, some variation of this:

                if (!md.hasModifier(J.Modifier.Type.Private)) {
                    J.Modifier mod = new J.Modifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, J.Modifier.Type.Private);
                    md = md.withModifiers(
                            ListUtils.concat(md.getModifiers(), mod)
                    );
                }

Which has me thinking about this in the general case. Currently there are visitors which new/build elements manually, e.g.:

rewrite/rewrite-java/src/main/java/org/openrewrite/java/ImplementInterface.java

Lines 46 to 52 in a8f820b

	c = c.withImplements(ListUtils.concat(c.getImplements(), J.Identifier.build(
	randomId(),
	format(" "),
	Markers.EMPTY,
	interfaceType.getClassName(),
	interfaceType
	)));

I wonder if there is specific circumstances where it's better to new/build than to use template.

With all this said, here's what I'm wondering:

• Is new'ing up/building out elements "manually" considered a "questionable" practice?
• In general, is it "cleaner" or preferable to us template(...)?
  • When, if at all, would it be "more appropriate" to new/build elements manually?

[tkvangorder]

While JavaTemplate is fully capable of doing these types of refactoring operations, I guess the answer is "it depends". We did not add any sort of coordinates for modifiers, because it just seemed like you could easily manipulate the modifiers via ListUtils.

As for implements: In a majority of the cases you are simply adding a fully-qualified class name, the one exception to this might be where you need to implement an interface that has type parameters.

While using JavaTemplate is easy (and fast), there is still some overhead because it is pruning an existing AST, inserting the template to create a synthetic compilation unit, compiling that code, and then extracting/merging the generated AST elements.

[jkschneider]

there is still some overhead because it is pruning an existing AST

I don't know that overhead should be something to be concerned about right now, especially given that template-based insertions are generally already guarded behind some sort of conditional checking to see that a change needs to happen. Probably premature optimization to worry about this much.