RefactoringWeston M. Price May 29, 2006 12:31 PM
Currently, on the 4.0.5 code base I have done the following:
Moved all pool related source into a newly created package
org.jboss.resource.pool
This removes the inner classes from the JBossManagedConnectionPoolMBean implementation and is overall, a cleaner way to organize things (at least in IMO).
I would like to do something similar with the jdbc supporting stuff (ie ValidConnectionChecker, ExceptionSorter etc, etc) and get that stuff out of the adapter structure.
I was thinking something along the lines of
org.jboss.resource.jdbc.support
and refactoring from there.
Thoughts?
1. Re: RefactoringWeston M. Price May 29, 2006 12:49 PM (in response to Weston M. Price)
New packages:
org.jboss.resource.jdbc.support
org.jboss.resource.jdbc.support.vendor
The former includes the
Cached*
BaseClasses/Interfaces
The latter
Vendor specific stuff.
2. Re: RefactoringAdrian Brock May 29, 2006 12:54 PM (in response to Weston M. Price)
You cannot change any interface or user level class.
You break somebody else's compilation.
That is unless you leave the interface in place and then
do:
public class ValidConnectionChecker extends org.jboss.new.location.ValidConnectionChecker
I'm against refactoring just for the sake of it.
i.e. no good reason like just changing names or packages.
There are some people who have access to the codebase that
I would like to go their IDE and put a big sticker over the refactoring
button saying "leave it alone and do some real work!". :-)
or
"leave it alone until you learn to be more careful about breaking the build!".
5.0.x (major version number change)
is the place where you can change things incompatiblity.
And then only if there is a good reason for it.
3. Re: RefactoringWeston M. Price May 29, 2006 1:06 PM (in response to Weston M. Price)
It's not refactoring for the sake of refactoring. In terms of the JDBC support, this should have never been in the "adapter" package to begin with.
As far as the 'real work' comment, not sure about that one :-) And as far as I remember I haven't broken the build...well, that one time due to the 1.4/1.5 stuff. Making this easier to read, understand etc, etc. may actually alleviate some of the 'People just don't grok JCA' mantra.
In terms of the pool classes, The JBossManagedConnectionPool/MBean remains in the same place. The only thing that has been moved is the BasePool and all it's children along with the management threads (PoolFiller, IdleRemover, ConnectionValidator) which should never be used directly anyway.
4. Re: RefactoringWeston M. Price May 29, 2006 1:10 PM (in response to Weston M. Price)
And, the build works fine (with the changes), testsuite runs fine etc. etc.
I do actually do these things before checking stuff in you know :-)
5. Re: RefactoringAdrian Brock May 30, 2006 5:25 AM (in response to Weston M. Price)
It wasn't aimed at you, more at the webservices project. :-)
My aversion to refactoring still stands.
If it doesn't serve a good purpose.
On the jdbc support classes, I don't see how these
aren't a part of the resource adapter anyway?
1) They are specific to jdbc
2) The rar wouldn't work without them
3) The rar shouldn't depend upon classes from the connection manager
There is a support jar
jboss-common-jdbc-wrapper.jar
that should really be in the rar as well.
The reason it is not, is because we have multiple versions
of the jdbc rar.
* This is legacy of the JCA1.0 packaging where you could have
only one outbound MCF per rar
* The HA rars only exist separately because they were initially
experimental
If you do move them somewhere else, the integration apis
should be in a ".spi" package so it is clear which interfaces
are supported for user integration. But that isn't something
we can do in mid 4.0.x branch.
I prefer the package name ".plugin" for implementations of the spi.
6. Re: RefactoringWeston M. Price May 30, 2006 10:48 AM (in response to Weston M. Price)
Right, 'refactoring' was probably a bad word :-) And I am in complete agreement with the frustration of doing an update and suddenly things are either completely removed, completely different etc, etc. Nothing worse than wasting an hour trying to figure exactly what happened.
I guess the comment was more directed towards improvements down the road. How we want to do things differently, how much of the JCA/MC stuff we can share across implementations. That sort of thing.
7. Re: RefactoringWeston M. Price May 30, 2006 11:09 AM (in response to Weston M. Price)
Our JDBC RAR's should really be collapsed into one RAR as you point out. This would clean things up tremendously I think. We could also remove the DummyResourceAdapter stuff at the same time.