[feature] copying JCodeModel

[glelouet]

I propose to add a new method in JCodeModel : copy(). This method returns a new CopiedModel that contains recursive copy of all the elements of the parent model. This CopiedModel also allows to translate the elements of the parent model into its own elements.

This method could be used with the preprocessors : instead of working on the original model, processors would be applied on the copy. If no processor is used, then no copy is done.

Working on it.

[phax]

That means a deep-copy of all objects and expressions... That's going to be a big one

[glelouet]

yes.

[phax]

Okay, than can we have quick chat on how to do it? I don't like Object.clone because you always need to cast. Can you do something with copy constructors? Or do your prefer a custom clone like in https://github.com/phax/ph-commons/blob/master/ph-commons/src/main/java/com/helger/commons/lang/ICloneable.java ?

[glelouet]

I'll make a branch.

I was more asking if there is a visible impossibility for this, as to avoid doing "a big one" for nothing.

The method in JCodemodel is just copy(), the ModelCopy extends JCodemodel and add various translate(..) to get the translation.

[phax]

One thing that strikes me, is that the basic types (like VOID or BOOLEAN) depend on the JCodeModel instance and all usages of those must also be transfered. That means that the new CM must be passed through quite deep

[glelouet]

The issue I have, is that some constructor eg Jatom() are protected.

[phax]

Just make them public. Or create factory methods

[glelouet]

NAh I put my ModelCopy in the same package.

The less modifications, the better.

[glelouet]

https://github.com/guiguilechat/jcodemodel/blob/copying/src/main/java/com/helger/jcodemodel/ModelCopy.java

[phax]

I though you add a copy-constructor or clone method in every class? Do I get you right - you manually copy the structure from the "outside" of the relevant class?

[glelouet]

yes, I just add one class that makes the copy in itself.

[phax]

Ah okay. But isnt't there a certain danger that you forget stuff?

[glelouet]

indeed.

[phax]

Than maybe for the expression stuff could you go for a solution like this: https://github.com/phax/jcodemodel/commit/ead1645d882ff301a530480c26e1e6d7b2c1348d

[glelouet]

No, because we need to translate all references .

[phax]

Okay, and what if IJObject gets a new method getClone (JCodeModel new) ?

[glelouet]

They must be able to translate their internal fields too. So they need to be getClone(ModelCopy). Also then their children may inherit the method and forget to duplicate their added field. Maybe I should use reflect.

[phax]

Reflection makes it even more difficult, especially if some byte code modifying frameworks like ByteBuddy or the Spring stuff is used, you might get weird errors. I would really prefer a clean, explicit way. Ideally with the respective code in each class separately. There are not too many derivations.

What could be an issue is the order of cloning of classes - because the references between created classes can only be copied properly, if the order is maintained. Maybe a separate "creationOrder" field is needed for classes?

[glelouet]

anyhow reflect can't be used since there is no empty constructor.

Not sure what you mean about issue of cloning classes. I cache the classes, when the class is missing I create it, cache it, then start building it. So there should not be a recursion issue.

I try to avoid making weird stuff in other classes, like creating code that is not necessarily relevant. If later the cloning costs too much dev time, we can just deprecate it ? I try to throw exceptions when I can to tell that a case is not caught.

[phax]

Okay - that sounds like a fair compromise. I like throwing exceptions :)

[glelouet]

I had to add

public Class <?> getReferencedClass ()
    {
        return m_aClass;
    }

in JreferencedClass, it had no public access.

[phax]

Such getters are always great. This is one of the big differences of this project to the original codemodel. If you find more of those - just add them.

[glelouet]

I did part of the translation of types. Still a lot to do ^^ https://github.com/guiguilechat/jcodemodel/blob/copying/src/main/java/com/helger/jcodemodel/ModelCopy.java

I use instanceof when I know the delegated method makes a hard check on the sub types, getClass()== when checking for a specific type to generate. This way if classes are added exception will be thrown.

[glelouet]

Another issue I got along is the caching of elements. I started by only caching the classes, for a deep copying recursive issue (what if a field refer to the class being copied ?) . But then I realized another issue.

Basically we want two things for the copying from a SRC model to a CP model:

1. once copied, no modification applied to SRC will impact CP, and reciprocally. eg renaming a variable, a method, deleting, creating. NO change at all.
2. once copied, any modification applied to CP will have the same exact result as when applied to SRC, and reciprocally.

The first part implies to copy everything, even the JAtom. The second part implies that the model"tree" is exactly the same, so if a node is present at two places, then its copy must also be present at two places. The only way to do that is to cache the nodes being copied. Since there will be deep copy that can produce recursion issue, the caching of a an IJObject copy must be done in a very strict way, that is always create a shallow copy, before storing it, then deep copy it so that the deep copy can reference it. It's a potential issue for items that have a constructor that needs a reference to another IJObject though.

[glelouet]

basically the translate(parametertype source) code is following :

• If the parameter type is interface, make a series of instanceof to delegate to a more precise subtype. end with throw exception in case new subtype was added.
• If the parameter type is abstract, same as interface
• If the parameter type is concrete class, then still try to delegate to subclass if any, then check for copy presence to return, if absent check same exact class, create a shallow copy, store the copy, make deep copying, return the copy. deep copy is made in separated methods because that can be shared for different part of code.

Then the "tree" of the method starts with interface IJObject, and create the whole interface subclasses translates (therefore only instanceof ) . Then another part of code is for each abstract class, becaue abstract classes belong to a tree (no diamond like interfaces) then I can create a tree from the highest non interface class that implement IJObject.

[glelouet]

https://github.com/guiguilechat/jcodemodel/commit/3851843c9b20fe68c7da8c6f094ac4d10496cfa8

Here I show how it is with the caching.

What remains is … well every todo ^^

[phax]

yes, to build the same structure you somehow need a state that know about all objects and if they are already cloned or not. So I think coming back to a clone (CloneState) method per object is the safest way to go. Or make everything Serializable - than write to a byte array and back to an Object - than you also get different instances...

[glelouet]

cloneState is the ModelCopy class.

I think you are right, that Serializable would be faster, more bug proof. But I'm not sure how it would handle the cases of static instances. Also I still need to have the translate() method, that translates an item from the source model into the copy model.

Also : In order to test the copy() method, I need to have a way to convert the model into a string (or at least a comparable item) . Do you think there is already such a method I could use ?

Typically the test would be (pseudo code)

cm=createCM();
copy = cm.copy();
assertEquals(cm.representString(), copy.representString());
for(Consumer<Model> m : modifications) {
  m.accept(cm);
  assertNotEquals(cm.representString, copy.representString);
}
for(Consumer<Model> m : modifications) {
 m.accept(copy);
}
assertEquals(cm.representString, copy.representString);

[glelouet]

java code for the test.

The interesting part is that it can be subclassed to test various implementations of copy and represent. Obviously it will fail with the toString() and return source; implementations ^^

public class ModelCopyTest
{

    @Test
    public void testCopy ()
    {
        JCodeModel cm = createCM ();
        JCodeModel copy = copy (cm);
        Assert.assertEquals (represent (cm), represent (copy));
        for (Consumer <JCodeModel> m : modifications ())
        {
            m.accept (cm);
            Assert.assertNotEquals (represent (cm), represent (copy));
        }
        for (Consumer <JCodeModel> m : modifications ())
            m.accept (copy);
        Assert.assertEquals (represent (cm), represent (copy));
    }

    JCodeModel createCM ()
    {
        JCodeModel ret = new JCodeModel ();
        return ret;
    }

    JCodeModel copy (JCodeModel source)
    {
        return source;
    }

    String represent (JCodeModel target)
    {
        return target.toString ();
    }

    public Collection <Consumer <JCodeModel>> modifications ()
    {
        return Arrays.asList (cm ->
        {
            try
            {
                cm._class (JMod.PUBLIC, "MyClass");
            }
            catch (JCodeModelException e)
            {
                throw new UnsupportedOperationException ("catch this", e);
            }
        });
    }

}

[glelouet]

In last patch I made that method abstract, and made a simple implementation with serialization to copy ; and a new StringCodeWriter extends AbstractCodeWriter for which toString() return the whole content of the files created from a JCMWriter.build(), appended with the name of the file to start.

When I try to make a more complex model, I get this error : Caused by: java.io.NotSerializableException: com.helger.jcodemodel.util.FSName

[glelouet]

I added serializable on a few classes.

this patch https://github.com/guiguilechat/jcodemodel/commit/682f0ef538aefa88bd7f697930c7ceb867a633b3 contains a little more complex example (create an interface, then create a class implementing that interface with a simple method) and still passes.

[glelouet]

https://github.com/guiguilechat/jcodemodel/blob/copying/src/test/java/com/helger/jcodemodel/AModelCopyTest.java

is the abstract method. As you can see the creation of the cm is pretty simple. Do you have a CM created by other tests that is more complex ? The goal is just to test the copying, so a JCM that contains as much possibility as possible would be good.

This class https://github.com/guiguilechat/jcodemodel/blob/copying/src/test/java/com/helger/jcodemodel/copy/CopySerialTest.java is the actual test. I think I should push the delegate in the parent method since the represent is pretty convenient.

[glelouet]

I lost the link between a source item (say JDefined class) and it translated version but it's much simpler. MUCH simpler. Don't merge the PR, just copy the interesting parts. lots of useless stuff.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.