Bug 489228 - Consider merging DatabindingAdapter and GenericDatabindingAdapter classes
Summary: Consider merging DatabindingAdapter and GenericDatabindingAdapter classes
Status: RESOLVED FIXED
Alias: None
Product: Viatra
Classification: Modeling
Component: Addons (show other bugs)
Version: 1.2.0   Edit
Hardware: PC Windows NT
: P3 normal
Target Milestone: 1.2.0M2   Edit
Assignee: Peter Lunk CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-08 11:22 EST by Peter Lunk CLA
Modified: 2016-03-31 07:15 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Lunk CLA 2016-03-08 11:22:44 EST
As the DatabindingAdapter<T extends IPatternMatch> class contains no additional features, if compared to the AbstractDatabindingAdapter, merging of these classes should be considered.

Also consider removing the template parameter from the DatabindingAdapter class, as none of the methods of this class benefit from this feature.

Both of these are in the same package and the DatabindingAdapter has no other inheriting classes, the merger therefore should not face any direct issues.

Naturally it should be discussed if these classes are used outside VIATRA and how much effort it requires to adapt to these potential changes.
Comment 1 Zoltan Ujhelyi CLA 2016-03-08 11:27:26 EST
Balázs, as an advanced user of the data binding API, what do you think, would such a merge cause issues with existing users of the API (considering, we add a rewrite rule in the migrator)?
Comment 2 Balazs Grill CLA 2016-03-09 04:28:17 EST
I see no reason to distinguish the two Adapters. Also, I would like to propose a few simplifications:

First of all, calling them Adapters is a bit misleading, as they've nothing in common with EMF adapters and have no listener semantics. They're basically property factories, which only need to be instantiated to call their non-static, otherwise stateless methods. I propose to use the same factory pattern as in databinding: a non-instantiable (final class with private constructor) class with static factory methods.

proposed API: MatcherProperties.value(IPatternMatch match, String parameterName)

As the match already has reference to the QuerySpecification, the parameterName can be validated programatically, and the appropriate ObservableDefinition can be calculated on-demand. It does not seem to be expensive to justify a pre-calculated map, as the property (being a reusable element) will potentially be instantiated once for each parameter.
Comment 3 Balazs Grill CLA 2016-03-09 04:34:42 EST
Rectification: the property should not be instantiated using the match element. It should use the IQuerySpecification:

IValueProperty property = MatcherProperties.value(querySpecification, parameterName)
property.observe(realm, match)
Comment 4 Zoltan Ujhelyi CLA 2016-03-17 08:37:18 EDT
Added related tickets to 1.2
Comment 5 Zoltan Ujhelyi CLA 2016-03-17 08:58:50 EDT
Could you look at these? Have fun. :)
Comment 6 Eclipse Genie CLA 2016-03-30 09:06:11 EDT
New Gerrit change created: https://git.eclipse.org/r/69522