elk icon indicating copy to clipboard operation
elk copied to clipboard

Layout option type registrations in ElkReflect happen too late

Open uruuru opened this issue 8 years ago • 6 comments

To support IProperty#getDefault() for types such as KVector or LinkedList, the types are registered with the ElkReflect class. The registration of many types happens in the constructor of the LayoutMetaDataService. This is, however, too late if someone calls the #getDefault() method on any property before the instance of the LayoutMetaDataService has been created (which is done lazily).

uruuru avatar Sep 07 '17 10:09 uruuru

We could move the initialization code to a static initializer block? Then it's executed as soon as the class is loaded.

spoenemann avatar Sep 11 '17 12:09 spoenemann

True, the question remains if that's early enough. To be save, the core.service plugin would have to be loaded in an earlier startup phase (in the eclipse context ...)

uruuru avatar Sep 11 '17 12:09 uruuru

The ElkReflect initialization code for any IDataObject should be written in a static initializer of that class, then you're 100% sure it's executed before you use any instance of it.

For the collection classes, we could put the initialization code into Property?

spoenemann avatar Sep 11 '17 13:09 spoenemann

The ElkReflect initialization code for any IDataObject should be written in a static initializer of that class, then you're 100% sure it's executed before you use any instance of it.

That would be a tremendously nice solution, but I'm not sure it would be enough. Retrieving a default value effectively results in a call to ElkReflect.clone(...). Here's the relevant part of that method's definition:

if (REGISTRY_CLONE.containsKey(clonee.getClass())) {
    return (T) REGISTRY_CLONE.get(clonee.getClass()).clone(clonee);
}

It first looks for a registered cloning function in its registree, and then uses that function to actually create a clone. Now the JLS defines in section 12.4.1 when static initializers are invoked: not when the class is loaded implicitly through import statements, but only once someone actually tries to access its members or create a new instance. Thus, my understanding is that the IDataObject would only register its cloning function after ElkReflect has already determined that none is registered.

We could still make this work with a small change, that would admittedly make it slightly more ugly. If we require data objects to have a constructor that doesn't take any parameters, ElkReflect could invoke that if it doesn't find a cloning function. This would give the data object a chance to register itself. Then, ElkReflect would make another attempt.

le-cds avatar Sep 11 '17 14:09 le-cds

Sounds feasible.

spoenemann avatar Sep 12 '17 06:09 spoenemann

I just wanted to start implementing this solution, but it turns out that I forgot one detail in my previous reply: invoking the default constructor is a reflection thing, and ElkReflect was built to avoid reflection since it causes problems with elk.js.

I'm postponing this for the moment until we find a proper solution. Getting property default to work properly is starting to really cause me headaches... :D

le-cds avatar Sep 13 '17 14:09 le-cds