lbeurerkellner / ttc2020

0 stars 5 forks source link

Review of Fulib #3

Open arturboronat opened 4 years ago

arturboronat commented 4 years ago

This solution encodes Ecore model instances as chains of events in order to use an event sourcing mechanism, with the command design pattern. Events are assumed to respect two principles: overwriting (the last edit to an object is the prevailing one) and commutativity (edits to different objects can be performed in any order). For the solutions, two editors (M1Editor and M2Editor) are used, one for each data model version involved in the corresponding task, each of which is able to apply modifications to its model instance depending on the modifications to be performed in each task. M1Editor subsumes the logic for creating model instances and for merging modifications from the migrateBack operation and M2Editor obtains objects from the migrate operation.

Expressiveness

The solution and functional test cases run properly. Test case task_3_M2_M1_M2_b is the only one that fails.

Comprehensibility

The solution presented in the paper lacks detail when compared to the code of the solution. For example, the implementation of the task Task_Fulib_M1_M2_M1 shows how command objects are sent to M2Editor. There seems to be additional code to be executed, like the creation of a container, that is not explained in the paper. There are many classes that have not been explained for each editor. Although these seem to be generated boilerplate code, the ones that are related to the solution could be explained. Overall, to have a better understanding of the approach, one needs to understand the FulibServiceGenerator, which is mentioned but not presented in the paper.

The solution involves changing the representation of the model instances (from the given Ecore models to the internal command representation in Fulib) in parse methods of Editors. Commands have a method run that needs to be implemented in order to create an EObject. However where is the information to be used to initialize that object coming from? That is, in listing 1, where do this.name, this.age come from?

Migration is performed in the methods parse of editors but there seems to be some 'parsing', for translating EObjects into command objects, in the run method, as well. This is explained in step 1.b) of Figure 4. So the use of the term 'parsing' is slightly confusing.

Bidirectionality

The solution provides support for concurrent changes in both editors via serialization and merging of commands objects. It is not clear whether unidirectional edits need to be implemented manually in the Command::run method, for each command class, and another time in the Editor::parse<Class> methods, for each command class, for each participant model (source and target). The overall net effect is that of a bidirectional transformation.

Re-usability

The solution relies on a data model that merges the different versions of the data models that participate in the different tasks, as source or as target. Only two editors are used and their corresponding parse methods contain migration logic. This means that considering more examples, involves both modifying the base data model, potentially adding more command objects and adding more logic in editors' parse methods.

Performance

Performance tests could be run in the solution. Performance is admittedly worse than that of the reference solution.

georghinkel commented 4 years ago

The paper shows how the case is tackled using Fulib.

Expressiveness

The command implementations are basically plain Java code so that Fulib is able to express quite anything here. However, the applicability is limited because the solution makes the assumption that users have control over editors.

Comprehensibility

In my impression, the Fulib solution comes with a lot of boilerplate code that conceal the places where the old model actually differs from the new model. It is close to impossible to retract from the code what the differences between V1 and V2 metamodels might have been or where exactly they are implemented.

Bidirectionality

The rules for forward and backward directions are entirely separated.

Re-usability

The solution reuses the rules for scenario 1 for later scenarios as it operates on a merged metamodel.

Performance

In my opinion, the provided test models are way too small in order to get any reliable finding with regard to performance.