[
https://issues.apache.org/jira/browse/ARIES-1062?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13648437#comment-13648437
]
TangYong commented on ARIES-1062:
---------------------------------
John,
> It would be much easier to compare these particular changes in a patch.
If finally you agree with the patch, pl.seeing it.
> Still, this is a delicate area of the code and we would need to be sure any
> changes here passed both the Aries tests and >
OSGi CT.
Yeah, I agree with you.
>See, for example, Effective Java, 2nd Edition, Item 60.
I have seen the item and in this case, you are right. thanks.
Tang
> org.apache.aries.subsystem.core.internal.Activator needs to be improved
> -----------------------------------------------------------------------
>
> Key: ARIES-1062
> URL: https://issues.apache.org/jira/browse/ARIES-1062
> Project: Aries
> Issue Type: Improvement
> Components: Subsystem
> Affects Versions: 1.0
> Reporter: TangYong
> Priority: Minor
> Attachments: Activator.java.patch
>
>
> Currently, org.apache.aries.subsystem.core.internal.Activator class and
> org.apache.aries.subsystem.core.internal.SubsystemResolverHookFactory class
> have some curious coding as following:
> 1. activate()
> "new Subsystems()" can not throw SubsystemException.
> 2. addingService method
> I suggest the following writing:
> @Override
> public synchronized Object addingService(ServiceReference<Object>
> reference) {
> Object service = bundleContext.getService(reference);
>
> if (service instanceof IDirectoryFinder){
> finders.add((IDirectoryFinder)service);
> return service;
> }
> else if (service instanceof Repository){
> repositories.add((Repository)service);
> return service;
> }
> else if (service instanceof Coordinator){
> if (coordinator == null) {
> coordinator = (Coordinator)service;
> }
> }
> else if (service instanceof RegionDigraph) {
> if (regionDigraph == null) {
> regionDigraph = (RegionDigraph)service;
> }
> }
> else if (service instanceof Resolver) {
> if (resolver == null) {
> resolver = (Resolver)service;
> }
> }
> else if (service instanceof ModelledResourceManager) {
> if (modelledResourceManager == null) {
> modelledResourceManager =
> (ModelledResourceManager)service;
> }
> }
>
> activate();
> return service;
> }
> 3. SubsystemResolverHookFactory class's constructor
> I have never seen the following and explicitly throw NPE
> if (subsystems == null)
> throw new NullPointerException("Missing required parameter: subsystems");
> I suggest that we'd better use RuntimeException or IllegalArgumentException
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
