[recommenders-dev] [models] Simplify configuration of ProjectCoordinateA

[Andreas Sewe <andreas.sewe@xxxxxxxxxxxxxx>]

This conversation started in Gerrit [1], but is by now probably more
suited for the list:

Olav wrote (in Gerrit):
> @Andreas: I thought about and would say: yes and no ;-) The
> ProjectCoordinateProvider uses the ProjectCordinateAdvisorService and
> add the cache (DependencyInfo --> Pc) functionality. Until here it
> sounds like a wrapper. But it also provide the mapping between
> workspace artifacts and DependencyInfos. The class has two
> responsibilities of that class and I would say that adding the
> configuration logic of the ProjectCoordinateAdvisorService would make
> it worse.
> 
> Also I would like to mention an other point: At the moment as user of
> the API you can get injected a ProjectCoordinateAdvisorService or/and
> a ProjectCoordinateProvider as well. The ProjectCoordinateProvider
> access the ProjectCoordinateAdvisorService inclusive the cache logic.
> The ProjectCoordinateAdvisorService access the advisors directly. I
> think this looks a little bit confusing and inconsistent.
> 
> I mentioned that because I thought about a solution to fix both: What
> do you think about create a wrapper for
> ProjectCoordinateAdvisorService which take care of the configuration
> AND the cache (DependencyInfo --> Pc) logic? The
> ProjectCoordinateProvider will still have the other cache and would
> use the wrapped ProjectCoordinateAdvisorService. I think we would get
> a better separation and could get rid of the inconsistens by using
> ProjectCoordinateAdvisorService without cache and
> ProjectCoordinateProvider with cache both.

OK, let me rephrase this to make sure we are talking about the same thing:

The IDE-idenpentent ProjectCoordinateAdvisorService is really just list
of advisors that can be reconfigured (advisor order can be changed) in a
*thread-safe* manner but that doesn't react to any UI events. The
Eclipse-specific ProjectCoordinateAdvisorService would then take care of
reacting to changes in the users preferences and also maintain a
DependencyInfo -> ProjectCoordinate cache.

This makes sense, in particular as UI events (somebody changing their
advisors preferences) would case *this* cache to be invalidated.

The second (Eclipse-specific) service would be the
ProjectCoordinateProvider. This class provides a second level of caching
(workspace elements -> DependencyInfos) as well as a nicer interface
(ITypes, etc. instead of DependencyInfos) but would not need to react to
changes in the advisor configuration. It would, however, need to react
to changes in the workspace.

This sounds very good to me. Or am I missing something?

Andreas

[1] <https://git.eclipse.org/r/#/c/18616/>
-- 
Codetrails UG (haftungsbeschränkt)
The knowledge transfer company

Robert-Bosch-Str. 7, 64293 Darmstadt
Mobile: +49-170-811-3791
http://www.codetrails.com/

Managing Director: Dr. Marcel Bruch
Handelsregister: Darmstadt HRB 91940