Here is my review before I'll submit the evaluation sheet this evening.
I was able to run the solution with the authors' fork, branch
SDMLibContribution
. The solution passes all tests except for these three:
testIncrementalInsertsDynamicConfig
testIncrementalOperational
testIncrementalRenamingDynamic
Since the forked project formats the models in text format all in one line (in
contrast to the main benchmarx project), it is hard to spot what the actual
problem causing the failures is.
Anyway, what I like with this solution is that it is really fast. Only the
BXtend solution is faster (about a factor of 7 for the batch cases and a factor
of 2-3 for the incremental ones), and most importantly, it doesn't seem to have
scalability issues when the model sizes grow.
Another good point is its conciseness. It consists of only about 100 lines of
code when counting just the transformForward()
and transformBackward()
methods and the few helper methods called from there.
An obviously weak point of the solution is that it is not bidirectional at all,
i.e., the forward and backward transformation are completely separate. The
only thing they have in common is that they work on the very same kind of
models. That said, this can be a reasonable choice in certain scenarios. In
this very case, in my opinion, the two preference parameters can be a hindrance
for a bidirectional solution because they only have a meaning in the backward
direction but their values must not affect the forward direction. Thus, I also
had to build one relation which explicitly dispatches on the transformation's
direction -- a thing you wouldn't want to have in a truly bidirectional
solution.
The visualizations of the pattern are nice and easy to grasp. However, the
Java code that you actually have to write could be a bit nicer. Do I see it
correctly that some methods like createCondition()
get a functional part (the
Condition object) and some text for the visualization which doesn't need to
have anything to with that Condition?
The decision to manifest the correlations between family members and persons by
extending the metamodel with explicit links is a good one which I sadly haven't
considered myself for the FunnyQT solution. However, I think the quite
operational and less declarative approach provided by SDMLib doesn't work for
more complex models. In this case, where we actually just deal with family
members and persons, it is quite feasible to use optional subpatterns and NACs
to distinguish the case where we already have a matching counterpart and the
other case, where we need to create one. But I assume that patterns would
quickly become unwieldy in real-world cases. As a transformation writer, I
don't want to have to encode this "check if exists or create anew" semantics
myself each and every time but want it built into the transformation language.
The solution paper suffices to explain the solution reasonably well, however it
doesn't meet the requirements mentioned in the case description. That is, it
doesn't mention which test cases it passes expectedly or unexpectedly, and
likewise it doesn't mention if the three failing tests fail due to a tool
limitation, a bug, or whatever. (Well, it actually doesn't mention that there
are failing tests at all.)
So all in all, the solution solves almost all test cases, it is concise and it
is really fast. However, it is not bidirectional at all.