[
https://issues.apache.org/jira/browse/FELIX-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14564724#comment-14564724
]
Thomas Watson commented on FELIX-4848:
--------------------------------------
The latest patch is incomplete, it is missing the ConsistencyCheck class. I
don't have a strong opinion on the need for this patch, but if it helps others
understand the code better then it seems like it could be a good thing. Not
seeing the complete patch I do have some concerns. Did you really just move
code from one class to another without making functional changes? Or did you
fix some bugs along the way? I ask because the changes to the Util class may
be problematic.
For getSymbolicName and getVersion you now use the osgi.identity namespace to
lookup the capability with the info. That is good, but at one point the
WrappedResource did not pay attention to the namespace param (see FELIX-4727).
I'm not saying the code should not use the namespace here, but it does require
that the input resources pay attention to the namespace (we certainly do in
Equinox, but I don't know about other users). But the other issue is that the
new impls of these methods will throw out of bounds exceptions if the resource
has no osgi.identity capabilities (e.g. for R3 bundles). Perhaps not important
to most, but Equinox still supports R3 bundles and I am pretty sure Felix does
also.
Once the complete patch is posted it is going to be hard to confirm that all
that was done is a refactoring of the code, without changing the functionality
and I worry about that based on the changes done to the Util class.
> Split ResolverImpl
> ------------------
>
> Key: FELIX-4848
> URL: https://issues.apache.org/jira/browse/FELIX-4848
> Project: Felix
> Issue Type: Improvement
> Components: Resolver
> Affects Versions: resolver-1.0.0
> Reporter: Christian Schneider
> Fix For: resolver-1.4.0
>
> Attachments: Dependencies in resolver package after patch.png,
> FELIX-4848-1.patch
>
>
> ResolverImpl currently contains about 2300 lines of code. That is way too big
> for a single class.
> I looked into it and found that the checkDynamicPackageSpaceConsistency and
> checkPackageSpaceConsistency methods and their dependent methods form a nice
> subset. I would move that into a class ConsistencyCheck.
> Both would share all of the inner classes of ResolverImpl but nothing else.
> So I think i would make sense to move these inner classes to separate files.
> These changes should nicely split the classes into
> ResolverImpl : 1400 lines
> ConsistencyCheck : 600 lines
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
