felix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bob Paulin <...@bobpaulin.com>
Subject Refactoring possibilities in the BundleWiringImpl for Weaving
Date Thu, 20 Nov 2014 17:39:27 GMT

Wanted to get some thoughts on some things in the BundleWiringImpl. 
There seems to be some very large methods in this class which makes it 
very difficult to write tests.  I wanted to get some opinions on 
breaking some of the methods up to improve readability and testability.  
The findClass method within the BundleClassLoader is a good example of a 
large method that could benefit from this.  I'd like to break down the 
method to better fit the specification around Weaving so that there are 
specific methods around Transform and Define phases of the weaving.

So for example it could be something like:
                    transformClass(felix, hooks, wovenClassListeners,
                             name, bytes);
                    catch (Exception e)
wovenClassListeners, wci);
                     /* Do locking code - omitted for breivity  */
                     try {
                         clazz = defineClass(felix, wovenClassListeners, 
wci, name,
                                     clazz, bytes, content, pkgName, lock);
                     } catch (ClassFormatError e) {
                         if(wci != null){
wovenClassListeners, wci);

Are there other arrangements that make more sense for this class or 
reasons just to keep it all in the same method?  I'm a big fan of 
smaller methods where possible since it helps with readability and 
testing.  I might be interesting to run the code through a static 
analysis tool such as SonarQube (I know some other projects build this 
into their Jenkins processes) for no other than to identify some spots 
that might benefit from some refactoring. Also I realized that I added a 
logger to the constructor of the BundleClassLoader only to realize that 
a logger was being exposed through the BundleWiringImpl class that's 
passed in the constructor.  Any thoughts on exposing the logger through 
a constructor instead? Looking forward to input on this.

- Bob

View raw message