[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [recommenders-dev] [models] Simplify configuration of ProjectCoordinateAdvisorService
|
Hi Andreas,
your summaryincludes everything I wanted to mention.
Best regards
Olav
Am 22.11.2013 14:47, schrieb Andreas Sewe:
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/>
