Hi Allen
Thanks a lot for the feedback!
Allow me some more comments and attempts at being a little clearer :-)
> I took a bit of a look at it and it looks fine, thanks for providing
> this example. It's a little hard to compare since it doesn't actually
> run properly, but I was able to get the basic idea.
Sorry about that. I jumped in the middle of some major code changes for
4.0 so I had troubles getting something stable to run, but yup, that
wasn't really the goal.
> As for my general
> thoughts on Spring vs. Guice ...
>
> http://code.google.com/p/google-guice/wiki/SpringComparison
>
> Guice seems to be the smaller, lighter, and more focused DI option and I
> strongly prefer their Module approach and their use of annotations over
> config files. The performance stuff sounds interesting as well although
> I don't consider it a huge issue. The benefit of Spring of course is
> that it offers more options in terms of non-DI stuff such as the mail
> sender and thread executor stuff.
>
Yes, I've tried to show in my sample, there's a lot more in Spring than DI.
Generally speaking and performance-wise, the only hit with Spring is at
application startup, so it's of very little concern in a web app world,
or systems such as Roller.
Actually, I've heard a lot of claims about Spring slowliness that have
been proven wrong with my past experiences, and honestly, the benchmark
code
(http://google-guice.googlecode.com/svn/trunk/test/com/google/inject/PerformanceComparison.java)
looks a little silly to me: it goes down in the guts of the Spring API
that no-one ever touches to benchmark some accesses or construction
patterns that the Spring factories implementation classes optimize,
bypassing that totally, which is the whole point of Spring providing
such a vast API and set of objects. There are many areas where
performance actually improves by using Spring since you can for example
lazy-load any bean you choose and don't have to manage any synchronized
access (or very few on implementation specific classes) or
caching-related things in the code.
I don't have exact figures for Roller but I noticed a 12s delay vs 8s in
Tomcat context loading using Spring.
Past that point, without using lazy-loading, I couldn't see any
difference, even when adding interceptors to log DAO or business
methods. You can "feel" beans being lazy-loaded (1/2s at worse?) when
switching that on but again, that doesn't matter since it's done once.
> * I definitely hate all the config files. I don't know if you separated
> them for example purposes, but you have like 7 config files and IMO that
> is way confusing. I would really only consider Spring if this was all
> merged into a single file or at least just one or two files.
The mail and database provider are in separated files to make sure that
modules that would only use the mail provider or database provider don't
load any unwanted/unnecessary bean by using only one file (a common
mistake with using Spring without lazy-loading, which I did when I
started playing with the Roller code for mail util).
Now for all the rest (all the XML for the implementation beans), it
really is just me and my coding style (I'm a "500 lines of functional
code in a single java class is probably more than you'll ever need" kind
of refactoring ayatohlah). It's very easy to have only 3 Spring files,
or even only 1. However, separating the business impl also helped me
switch from one back-end to another in one line while
testing/experimenting.
Using config files is indeed a different approach than using
annotations. I am personnally a fan of XML and I much prefer
configuration files to instrumenting code with annotations or other
comment tags that are tool-specific, requiring for example a build cycle
to fix a setting (such as the hibernate tags). I also tend to dislike
binding business objects to anything else than the API they belong to.
However, that's a very philosophical debate that we'll keep for another
time.
> * The property file injection thing is interesting and seems powerful,
> but i'm worried it's not entirely appropriate for us. for one, it means
> that there are now 2 ways to get properties out of those config files
> (via Spring and RollerConfig) and that complicates things. second, lots
> of the the stuff in those configs doesn't apply to the backend, so it
> seems weird to support duplicated config mechanisms for the backend
> wired by Spring and for the rest of the code. In your example you
> didn't include the fact that a 3rd way to specify roller properties is
> by supplying a path to a file via a jvm option, so you would need to
> support that as well.
As I mentionned in my previous mail, I didn't went as far as I wanted to
with property files.
Of course, RollerConfig and RollerRuntimeConfig should use the same
mechanism and sources,
which can easily be done by modifying their implementation. Ideally,
there should be few to no references to static methods in a code that is
using DI as its core (each property a class needs should be injected, so
as to help testing and re-use) but we can get around easily with that
since RollerConfig is pretty much everywhere. I didn't provided a
RollerConfig implementation that reads its configuration from Spring for
lack of time, and fear to break too much stuff in the same batch.
> In your example you
> didn't include the fact that a 3rd way to specify roller properties is
> by supplying a path to a file via a jvm option, so you would need to
> support that as well.
I wasn't aware of that option, but that should be a breeze to add. That
won't probably even require writing any code.
> * The way you did the persistence stuff is probably what i liked least.
> I am not in favor of wiring up our backend in a Spring specific way
> using things like Springs HibernateUtils, etc. I already think that ORM
> solutions provide enough of a challenge in terms of figuring out exactly
> how a call to save an object turns into actual jdbc actions and tracing
> that process becomes much more complicated when you mix in yet another
> dependency like Spring. So I am not a fan of introducing Spring in this
> area and what we have now works fine so there is no reason to change it.
> All that being said, this stuff has nothing to do with using Spring DI
> for the backend and is not required as far as I know so it's just
> something I would cut out.
Indeed, there is no reason to change the existing and the "hibernate2"
package is a scary, ugly and bad attempt at showing how transaction can
be moved to yet-more-XML-headaches and replace home-grown "strategies".
Ideally, it is best to use Spring's HibernateCallback(s) and
HibernateTemplate, which is one area where you want to tie an
implementation layer with Spring. But with those, you usually never have
to manage any transaction aspect, unless you do very complicated and/or
unlawful things with your SQL/Hibernate code, or need to support
transactions other multiple datasources. It also allows for using JTA by
just switching it on.
The business impl layer of this project
http://denis.balazuc.net/roller/blug-project-demo.zip
is probably a much better example of a simple DAO layer using Spring
with Hibernate.
Basically, HibernateXXXImpl (or HibernateRollerImpl - see below) can be
taken as is, have its strategy configured and injected through Spring,
and off we go.
> * You suggested getting rid of the XXXRollerImpl classes and just using
> a generic RollerImpl class but that sounds like a bad idea in my mind.
> The purpose of having those classes is to force a persistence strategy
> as a whole meaning that a JPARollerImpl should only allow injection of
> JPAXXXManagerImpl classes. From an api point of view it would be very
> silly to be mixing JPA and Hibernate code together at runtime.
You could go either or both ways actually.
Injecting a XXXRollerImpl in RollerFactory, or get rid all along of the
XXXRollerImpl really is a design-decision. My only reason was to remove
classes that do some wiring by code, managing their own singletons to
show how it gets simplified. It surely makes little sense in the
existing implementation. However, using Spring, it's a matter of
defining a "prototype" (or "abstract" or "template") bean and moving
configuration around, as long as you have the proper setters and getters.
> Like the
> last point, I think this is not something that really has a bearing on
> whether or not Spring DI could be used, but it's something that I am not
> a fan of in your particular design. Also related to this one is that I
> am 100% against allowing for public setter methods in the Roller
> interface or it's implementations. I mentioned this before, but the
> Roller instance should be immutable as far as the api goes and I think
> we need to enforce that when we do this DI work.
>
I could not agree more that setters have nothing to do in the Roller
interface. However, If you're using setter injection, you need public
setter (getters aren't necessary if not part of the implemented
interface) in interface implementors. I am usually against public
setters when it's not needed, but once you have DI in mind while
designing/coding, there's rarely any misuse of it. You can opt for
constructor injection, but unless the whole design is 100%
interface-driven, it makes it harder to integrate interceptors, use
dynamic proxies or reflection without a default constructor.
When I started working using DI, having public setters and getters used
to itch me a lot (maybe less than protected member variables though),
but since the "javabean-like" approach gets on everything, from Struts
action forms to Spring beans, it's now became the usual pattern and at
the end of the day is much cleaner than using protected members which
access can't be traced to avoid getters/setters (even protected), and
allows for a lot more fun such as mocks/stubs/etc.
> * The thread manager stuff is probably the most interesting piece in my
> mind and I wouldn't mind making use of Spring's built-in executor
> service stuff. The problem though is that we don't really have much
> need to do this 'cuz our code is fine as it is, and the other problem is
> that translating our config file properties into Spring has proven
> challenging with the Acegi stuff and I am hesitant to try.
The ExecutorService is actually a java.util.concurrent implementation
that got switched in Roller between two updates I did of the code IIRC.
All I did is add a constructor to inject the service, as I did for all
other XXXimpl, comment out my current Spring conf, switch it to
injecting an ExecutorService using the parameters I gathered - that's
all....Yup, that easy - and that was fun to do! :-)
It's actually a good example where Spring can really make a difference
with its possibilites. You can inject anything from a flat static
implementation of an interface, to an EJB, and this totally
transparently at a switch of a config. With Spring's use of interceptors
and dynamic proxies, you can go extremely far.
I'm currently working on a well-known brand of J2EE 1.4 app servers IRL
and with current threadmanager and scheduling implementation, I am not
sure I would not get barked at and asked to use a WorkManager if I were
to run Roller tasks on them. Being able to not touch the Roller code
(maybe provide my own external package) and change one bean using a
config file to make use of those server's built-in TaskScheduler(s)
would be a very good thing.
Aside from the rich set of services that Spring offers, one of my main
argument in favor of Spring for Roller is actually that it's already
used in Roller with Acegi, which makes the config changes in the webapps
and build files quite minimal. Also Struts/Struts2 modules already could
benefit from it: actions that want mail senders, or filters, or
renderers, all this may be injected through Spring extremely easily with
the Struts plugin. I was actually surprised that it wasn't already that
way and that they were still hard-wired dependencies on various services.
As for translating configuration files, I'm looking for something I
could help on, show me where to look ;-)
It is true that the learning curve with the Spring config is steep at
first but after the getting-used-to phase, it's easy to navigate between
the Spring API and the XML to change pretty much any aspect of an
application at a glance.
> So overall my basic feeling is that Guice provides the nicer option
> specifically for DI and I love that it's all done via java code and
> annotations and that the way you define Modules provides some compile
> time checking. Spring would work fine for DI, but I am still just luke
> warm on the config file wiring, so if I was making a decision now I
> would go with Guice.
>
As I mentionned earlier, I much prefer configuration files to
instrumenting code with annotations but it all boils down to design
decisions. Spring makes sense since it's already in Roller and allows
for all those neat services and helpers that it comes with on top of the
DI way that Roller is taking. It's worth the XML headaches IMO (I can't
legally ship aspirin from here...). In the end, a good Spring
deployement should hide the complexity of its configuration to end-users
or users of the API but may go as far as one wants in decoupling code.
If you want more Spring magic to be shown (I haven't covered Struts2
yet!) or someone to do any config macarena you hate, I'd be happy to do
more. If you give the go on Guice I'll just read more on and get my
hands deeper on it, but in both cases I'd love to help.
Thanks again for the great feedback.
Denis Balazuc
"A good Spring implementation is indistinguishable from magic."
No...Wait...That doesn't sound right...
Allen Gilliland wrote:
I took a bit of a look at it and it looks fine, thanks for providing
this example. It's a little hard to compare since it doesn't actually
run properly, but I was able to get the basic idea. As for my general
thoughts on Spring vs. Guice ...
From what I have seen from Spring and Guice so far I would basically
agree with what this article says ...
http://code.google.com/p/google-guice/wiki/SpringComparison
Guice seems to be the smaller, lighter, and more focused DI option and I
strongly prefer their Module approach and their use of annotations over
config files. The performance stuff sounds interesting as well although
I don't consider it a huge issue. The benefit of Spring of course is
that it offers more options in terms of non-DI stuff such as the mail
sender and thread executor stuff.
As for some of my thoughts on what I looked at in your code ...
* I definitely hate all the config files. I don't know if you separated
them for example purposes, but you have like 7 config files and IMO that
is way confusing. I would really only consider Spring if this was all
merged into a single file or at least just one or two files.
* The way you build and configure the mail and db providers is
interesting and could be useful, but I don't think it's a significant
improvement over what we already have.
* The property file injection thing is interesting and seems powerful,
but i'm worried it's not entirely appropriate for us. for one, it means
that there are now 2 ways to get properties out of those config files
(via Spring and RollerConfig) and that complicates things. second, lots
of the the stuff in those configs doesn't apply to the backend, so it
seems weird to support duplicated config mechanisms for the backend
wired by Spring and for the rest of the code. In your example you
didn't include the fact that a 3rd way to specify roller properties is
by supplying a path to a file via a jvm option, so you would need to
support that as well.
* The way you did the persistence stuff is probably what i liked least.
I am not in favor of wiring up our backend in a Spring specific way
using things like Springs HibernateUtils, etc. I already think that ORM
solutions provide enough of a challenge in terms of figuring out exactly
how a call to save an object turns into actual jdbc actions and tracing
that process becomes much more complicated when you mix in yet another
dependency like Spring. So I am not a fan of introducing Spring in this
area and what we have now works fine so there is no reason to change it.
All that being said, this stuff has nothing to do with using Spring DI
for the backend and is not required as far as I know so it's just
something I would cut out.
* You suggested getting rid of the XXXRollerImpl classes and just using
a generic RollerImpl class but that sounds like a bad idea in my mind.
The purpose of having those classes is to force a persistence strategy
as a whole meaning that a JPARollerImpl should only allow injection of
JPAXXXManagerImpl classes. From an api point of view it would be very
silly to be mixing JPA and Hibernate code together at runtime. Like the
last point, I think this is not something that really has a bearing on
whether or not Spring DI could be used, but it's something that I am not
a fan of in your particular design. Also related to this one is that I
am 100% against allowing for public setter methods in the Roller
interface or it's implementations. I mentioned this before, but the
Roller instance should be immutable as far as the api goes and I think
we need to enforce that when we do this DI work.
* The thread manager stuff is probably the most interesting piece in my
mind and I wouldn't mind making use of Spring's built-in executor
service stuff. The problem though is that we don't really have much
need to do this 'cuz our code is fine as it is, and the other problem is
that translating our config file properties into Spring has proven
challenging with the Acegi stuff and I am hesitant to try.
So overall my basic feeling is that Guice provides the nicer option
specifically for DI and I love that it's all done via java code and
annotations and that the way you define Modules provides some compile
time checking. Spring would work fine for DI, but I am still just luke
warm on the config file wiring, so if I was making a decision now I
would go with Guice.
Thanks again to Denis for providing the Spring example, it's much easier
to do a comparison when we can see both options :)
-- Allen
