Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 335089

Summary:	[generator] use import manager for reference to factory class
Product:	[Modeling] EMF	Reporter:	Knut Wannheden <knut.wannheden>
Component:	Core	Assignee:	Ed Merks <Ed.Merks>
Status:	CLOSED FIXED	QA Contact:
Severity:	enhancement
Priority:	P3
Version:	2.7.0
Target Milestone:	---
Hardware:	PC
OS:	Mac OS X - Carbon (unsup.)
Whiteboard:

Description Knut Wannheden

2011-01-22 09:28:19 EST

I am using the MWE Ecore generator component which by cleverly overriding the EMF code generator's ImportManager manages to apply the generation gap pattern to the EMF generated code.  See also http://git.eclipse.org/c/emf/org.eclipse.mwe.git/tree/plugins/org.eclipse.emf.mwe2.lib/src/org/eclipse/emf/mwe2/ecore/EcoreGenerator.java for implementation details.

Unfortunately this "trick" only works for the EClasses of an EPackage. It would be nice if this could also be applied to the generated EFactory implementation class, as this would allow overriding the serialization code for the package's data types in a purely non-generated class.

For this to work the generated singleton in the factory interface would have to use the ImportManager. So in FactoryClass.javajet replacing the line:

	<%=publicStaticFinalFlag%><%=genPackage.getFactoryInterfaceName()%> eINSTANCE = <%=genPackage.getQualifiedFactoryClassName()%>.init();

with something like:

	<%=publicStaticFinalFlag%><%=genPackage.getFactoryInterfaceName()%> eINSTANCE = <%=genModel.getImportedName(genPackage.getQualifiedFactoryClassName())%>.init();

should do the trick.

Comment 1 Ed Merks

2011-01-22 13:07:57 EST

Hmmm.  We are "cleverly" trying to hide this implementation detail as much as possible.  Adding yet another flag to the GenModel seems yucky too.  I suppose we might control the choice with a system property; also yucky... :-(

Comment 2 Knut Wannheden

2011-01-24 15:58:46 EST

Hi Ed,

I thought about using dynamic templates to work with a custom FactoryClass.javajet as I proposed. But that of course doesn't work very well as the MWE workflow is executed inside a standalone Java application... So for now I've ended up using the standard EMF generation pattern (i.e. generated code with manual modifications in protected regions).

What would be the problem with using an additional GenModel#getImportedName() in the FactoryClass.javajet template?

For my use case I think it would actually even be better to instead replace the following line (122) in FactoryClass.javajet:

	return new <%=genPackage.getFactoryClassName()%>();

with:

	return new <%=genModel.getImportedName(genPackage.getFactoryClassName())%>();

Comment 3 Ed Merks

2011-01-24 16:10:41 EST

We already have a getImportedFactoryClassName method so we could easily use that.  

The point I didn't make clear is that we *deliberately* don't use this so that we don't end up with a import of an implementation class in the API.  We're trying to hide that implementation detail as much as possible...

Comment 4 Knut Wannheden

2011-01-25 00:17:16 EST

OK, I get your point. But in that case my second proposed solution (see comment #2) should be OK, as that only affects the constructor call in the implementation class. So I would end up with something like this:

src-gen/foo:
public interface FooFactory extends EFactory {
    FooFactory eINSTANCE = foo.impl.FooFactoryImpl.init();
}

src-gen/foo/impl:
public class FooFactoryImpl extends EFactoryImpl implements FooFactory {
    public static init() {
        ...
        return new FooFactoryImplCustom();
    }
}

src/foo/impl:
public class FooFactoryImplCustom extends FooFactoryImpl {
}

Comment 5 Ed Merks

2011-01-25 14:11:03 EST

The change is committed to CVS for 2.7.

Comment 6 Knut Wannheden

2011-01-25 16:25:36 EST

That was quick!

It works like a charm. Thank you!

(In reply to comment #5)
> The change is committed to CVS for 2.7.

Comment 7 Ed Merks

2011-05-10 11:57:43 EDT

The changes are available in EMF 2.7 M7 or an earlier build.