Spring code remarks

[Alarmnummer]

I have a few questions/remarks about the Spring code.

The most important remark, why doesn`t spring do any checking on arguments? If you don`t add checks you can`t garantee that an object is in a valid state, and this makes it very difficult to find bugs. A bug in an object can be caused by faulty input from a totally different object and this makes it a nightmare to find the cause. Also bugs can sneak into the system without being detected for a long time.

Another remark: why do you extend from classes that don`t belong in the class hierarchy but are only used internally. An example is:
http://www.springframework.org/docs/...ctoryBean.html
that extends from ArgumentConvertingMethodInvoker. This is bad programming practice because it makes a class harder to understand and creates unwanted public methods. This should have been an internal class.

Last Remark: objects and classes depend a lot on eachother. Therefore it is very difficult to use parts of Spring and skip others. I had to integrate Spring with maverick and the platform we use at our company. In the beginning I tried to use all the Spring functionality... why write what already is written and tested. But time after time I find myself jumping and jumping through classes and finding al lot of dependencies. Finally I came to the conclusion that writing it myself was quicker than reusing Spring classes.

[Alarmnummer]

Not all at once But nobody up to it?

[Jon Chase]

Actually, Spring's code is some of the best written you're going to find anywhere - not just open source, either.

Download IBM's Structural Analysis for Java tool (http://www.alphaworks.ibm.com/tech/sa4j) and try doing some code analysis if you're really
motivated. Then try the same analysis on a project like Hibernate. Even in Hibernate's case (which is very well written too), you'll find
that there is a central dependency on the Hibernate "Session". Spring doesn't have a central dependency like that - even the ApplicationContext,
which is what Spring is basically built on, has VERY few dependencies on it for all that it does.

As for why it doesn't do much (it does do some...) arguement checking - most libraries don't do that much arguement checking anyway, so why
should Spring be any different? It throws helpful exceptions. And the frameworks and other libraries that it uses (i.e. Hibernate, Ibatis, JDO,
Quartz, etc.) probably don't do much arg checking either. I'd say most people's code doesn't do consistent arguement checking either...like
mine.

I'd step lightly before remarking on Spring's code quality - just think of how many things Spring can actually do - all in one library - most
other projects that attemp that scope don't do so well because their architectures aren't up to the task. I'd say Spring is a winner in the
"design and implementation" category.

Hope this helps!
Jon

[Alarmnummer]

Actually, Spring's code is some of the best written you're going to find anywhere - not just open source, either.

As for why it doesn't do much (it does do some...) arguement checking - most libraries don't do that much arguement checking anyway, so why
should Spring be any different?

I find this a bad argument. If everybody does it.. should you too? If find argument checking very valuable because it makes me think about the fine details of my contracts (my documentation). What is allowed.. what not..

And it helps me to find bugs sooner.. So I can`t see why you wouldn`t check arguments apart from the fact that it doesn`t make the code any prettier.

It throws helpful exceptions.

As soon as it checks for exceptions the messages are good. But there are places where no argument checking is done. This makes it more difficult to understand what is allowed.. and what is not.

And the frameworks and other libraries that it uses (i.e. Hibernate, Ibatis, JDO,
Quartz, etc.) probably don't do much arg checking either. I'd say most people's code doesn't do consistent arguement checking either...like
mine.

Well.. mine does.. I try it sometimes (without checking) but I find it to helpfull to find bugs, and I find it too helpfull to know how I should use my libraries.

I'd step lightly before remarking on Spring's code quality - just think of how many things Spring can actually do - all in one library - most
other projects that attemp that scope don't do so well because their architectures aren't up to the task. I'd say Spring is a winner in the
"design and implementation" category.

I never said that Spring is badly written, but there are always things that could be improved. And the argument checking and strange parent classes are things that could be improved.

[Keith Donald]

In general, we are doing more argument checking and assertions now. You'll notice the use of the util.Assert class for this. If you have a specific case where an arg invariant is not validated like it should be, let us know: open a JIRA issue.

[Martin Kersten]

Actually, Spring's code is some of the best written you're going to find anywhere - not just open source, either.

Wow that statement gave me a good lough. Here is why:

Ok I am currently implementing a metric and dependency analysation tool myself as a Eclipse plugin (my final exams work). You should check out the following classes, before you blame hibernate or other project or before you rephrase this non-excuse again.


HtmlUtils:

- check out the if clauses. The worst abuse of the countine statement I have ever saw. Check also the code duplication within this class.
- It has a commulative SLOC metric of 429! (SLOC is counting the ';' instead of purly counting the code lines and is much more stable according to the formating of code).


BeanWrapperImpl:

- Check out the huge constructor. This is a clear case for composition.
- It gets fine McCabe (Cyclomatic Complexity) measures, too. The devs should apply many extract method refactoring for this class
- Since it playes a center role, it is way to heavy.
- SLOC: 309!


DefaultXmlBeanDefinitionParser:

I am on it. I have a good solution, but it is not completed yet. I posted some questions on the dev-mailing list - since I got no reply, I delayed the work on this class(es). But I will get back to it soon. I will contribute a diffrent solution (static AST based) .
The current implementation is somewhat about 45KB in size. I have core packages in some of the projects I apply to it, which are much smaller.


My overall critic on the Spring code goes to:


* Packaging (Yeah I keep repeating this):

I did a test repackaging according some issues:

The following project/subproject structure would find better:

org.springframework,
org.springframework.j2ee
org.springframework.db
org.springframework.web

I have used the Java J2EE definition to pack the j2ee modul. I placed the transaction subframework within the j2ee subproject since it is considered to be a J2EE component but maybe this can be viewed diffrently.

* if statements:
if-else is a friend! The code should be refactored according to most of the if statements. Also the slicing is mostly wrong in terms of importance and code duplication.

* huge methods
Some methods should be extracted more methods. The methods should aim for a McCabe of <=5 (in special situations <=8 ) and a SLOC of <=5 (special <=10).

* Too many inline documentation
You see way to much inline documentation. There are classes where one third up to one half of the code is about documentation (not meaning JavaDoc on public methods).

* Too many helper methods
They should use fascades more often.

* Replicated functionality:
I think the application context and the bean factories have equal repsonsibilities. (but I am left alone on that hill :-)).


* Replicated lines of code
This is a pure blocker. Nearly every class I've looked at (I mostly look at the complex ones) contains many parts, where I would instantly start to refactor to avoid duplicated code. Check out the HtmlUtils class, which is a prime example about what I mean. Also doing a refactoring of the DefaultXmlBeanDefinitionParser (it is more a reimplementation to be honest, since the code drove me that way). I know, how many lines where actually duplicated within the private methods. Check it out and take a look.


* Inconsistent Typehierarchies:

There is sometimes an abuse of interfaces happening in my point of view. Check out the ApplicationContext. This is a hierarchical bean factory (so ApplicationContext can be used as bean factories directly) and also a hierachical application context. You have a parent factory and a parent context. This does not sound reasonable. In this case I consider the use of a BeanFactory as an implementation detail of the application context (beside the replicated repsonsibilities / features). Stick to composition rather than polymorphism... .


* Too many inner classes
Most of the inner classes are specializations of common types. I would advise to extract most of the inner classes and repack the packages. You will find some structural improvements, I am sure. Also some creational code can be moved to factories, where it belongs - in my oppinion -. Too many details!


Summary:

I find that the Spring code is not agile enough to meet modern coding standards. It suffers from some really huge and monolitic implementations. There should be a favour about objects instead of code duplication.

And as always, I am a happy user of the Spring framework, since its 1.0 release. But I am a guy, who likes to review the library code, he is working with. This is, why I love open source. But the Spring code does not provide that much readability like it should.

If I have repeated myself, I am sorry, but It looked like a good opportunity to throw my oppinion about the code quality in the ring again.


Cheers,

Martin (Kersten)

[Juergen Hoeller]

Martin,

In general, I do appreciate straightforward in-the-face thoughts on Spring's code base. There is always room to improve, so there is always a need for people pointing out weak spots.

However, the introduction to your reply here shows an offensive tone that I do not appreciate at all. This style hasn't served you well in the past few weeks either. Hint: If you want to be be constructive (which I assume you want), take other people serious, and try to understand where their opinions and decisions come from.

Do me the favor and have a look at the Hibernate 2.1 source code, the Struts 1.1 source code or even the JDK 1.4 source code. These references are not "excuses"; having a look at those will just give you an indication about the average code quality out there, in particular about the average quality of grown code.

In general, you completely neglect the fact that code grows and evolves. The challenges in evolving a large codebase over years are very different from the ones in designing a software system from scratch. This is not only about the ubiqitous burden of backwards compatibility; this is also about architectural decisions in a constrained environment.

Regarding your concrete complaints: I am aware that HtmlUtils is not perfect. But it does work sufficiently well, and it is a mere helper for Spring's web view support. It is by no means central to the overall framework, so we don't invest particularly large amounts of time in improving its internal implementation. Of course, it can be improved, no doubts about that - but this is by no means vital for overall Spring.

BeanWrapperImpl is one of those classes in the Spring codebase that have grown a lot over the years (in this case even since 2001). I fully agree that this class is a candidate for refactoring. But you seem to assume that we can refactor this whenever we want: This would be true in an academic world. Unfortunately, we cannot risk breaking subtle semantics in current behavior; this makes refactoring a major effort.

DefaultXmlBeanDefinitionParser is not perfect either, fully agreed. It's essentially by-hand DOM-based XML parsing. However, all this class does is parse Spring's XML bean definition files into BeanDefinition objects for a BeanDefinitionRegistry. While this is of course widely used, the actual implementation is not as important as you might think. For example, this code is only run on startup, never during operation of an application.

Let me reiterate that I do by no means object to reworking the implementation of those three classes. My main point is that this is not of incredibly high priority for the overall framework - in contrast to the impression that you leave. These classes might have been the spots that you concentrated on, but this doesn't mean that they are as central to overall Spring as you believe.

Regarding the packaging: Well, as you say, you keep repeating this, but that doesn't make your understanding of Spring's packaging any better. For example, the transaction framework is certainly not J2EE, but arguably the web framework is... your choices are quite arbitrary there. Packaging based on such coarse categories is hard to get right over the long term, which is why hardly any framework does it.

I also find it quite funny that you complain about too much inline documentation. I believe that such documentation helps a lot in understanding what's going on in the implementation. Javadoc on public methods does not help there at all. In my opinion, the lack of inline documentation is a major issue with Hibernate's source code, for example.

I find that the Spring code is not agile enough to meet modern coding standards.

Quite a bold statement. Actually, a very bold statement. Do you really think that your personal view is the single definition for "modern coding standards"? And do you really think that your bits-and-pieces analysis of classes that you consider important is sufficient for judging the overall Spring code quality?

Juergen

[Martin Kersten]

However, the introduction to your reply here shows an offensive tone that I do not appreciate at all. This style hasn't served you well in the past few weeks either. Hint: If you want to be be constructive (which I assume you want), take other people serious, and try to understand where their opinions and decisions come from.

Well I still try to fine tune things. I sometimes sound rude I know, but it is mostly because of a disappointing initial situation, a lack of experience in talking to not-well-known people about code by using a language, I still lack sharp situational knowledge about using the right words at appropiate moments. Also for the phrases 'best written' and 'better than hibernate', I find this most offensive and most arrogant, too. But I should have not given in that way either.

I don't refer myself as a person getting into stress mode that easily. But I reviewed quite a lot of Spring code and sometimes I just ended up with saying 'uff, thats complex, looks like more fluff than stuff'. And this happend quite often for my last attempts to understand certain Spring code parts. I know how hard it is to write a framework. I know what a relieve a reimplementation phase can be.

But the quoted statement of 'best written' was something that kept me pondering. You know I mean the word best. Best written code is self-explaining code. And doing some reviewing myself, I have a mixed feeling about this and I can say that there is a 'better is possible' sign tapping on some of the parts of the Spring code-base.

Also another thing is, that I am not a native english speaker (like everyone already knows, I am sure). I still would say that my English is beyond the level I need in my next 20 years. For example the word 'abuse' may be misleading. We use it here everytime, someone uses something in a inappropiated way. We speak the phrase 'Missbrauchen' in german, which can be indeed translated into abuse, but we use it with a more diffrent meaning and a more relaxed attitude.

Sure I can understand that abuse may sound rude - thinking carfully about it, it is quite obvious - and I will hardly try to drop this habit. Especially talking with people who don't know me. The problem is that I followed most of your discussions within the developer mail-list since my first hit with the Spring framework. So I still have the feeling raising a voice in a common area of beloved friends.

We also dont share a common culture of talking. I dont like some of the phrases my colleagues use. You know something like 'this is crap', but we use 'this smells'. But yet again this is something that I should stop instantly. But this has become a well trained habit over the years and I can not change it over the night. If I would know the the right button to press, I would press it for sure. So I really appreciate your clear speach.

So to summarize, we do not have a tight friendship relation or something similar. So I should watch my language and also by thinking about it, I just realised, that we are actually talking in the public. Someone might google my path and it isn't a good reputation to sound rude. Also It isn't by far not a good commercial for the Spring framework anyways. If someone sees this post, he might think something that I wasn't intended to.

So to make the following clear:

Your Spring framework had saved me countless hours of working time. I was fiddeling with CMP and EJB, just when Hibernate and Spring came along. So I guess you can imagen what a relief it was.

So I cover my head with ash. I didn't ment to scratch the reputation of you, your fellows or the Spring framework as a whole. And if I am just about to sound a way too rude or direct, please help me stear my roaring car of enthusiasm on the right side of the road. So thanks a lot, you wouldn't have told me by using the words you used, if you wouldn't want to give me a helping hand. Thanks Juergen!


But I also want to add that using a phrase like 'best written' is also rude. Maybe 'better written than Hibernate' would be a more polite anyways, but I wouldn't sign it also. I said Spring isn't best and and you stated 'we are better than hibernate'. I would suggest that both statements are a subject of unnecessary offence. Also I wouldn't buy the 'best' anyways. It's like watching a pointless commercials in TV about a washing agent. You hear that the washing agent will make your clothes perfectly bright, but you know that the next version of product will be reprased as make your clothes even more brighter than before. So it really gave me a true laugh mixed with some bitterness. Not a loud one. It was more like smilling and thinking: 'if you would know, what I went through, lately'.


But back on something more productive:

Please take a look at this piece of code:

if (c == 34) {
writeDecimalReference(c, escaped);
continue;
}
if (c == 38) {
writeDecimalReference(c, escaped);
continue;
}
if (c == 60) {
writeDecimalReference(c, escaped);
continue;
}
if (c == 62) {
writeDecimalReference(c, escaped);
continue;
}
if (c >= 160 && c <= 255) {
writeDecimalReference(c, escaped);
continue;
}
if (c >= 338 && c <= 339) {
writeDecimalReference(c, escaped);
continue;
}

This is just an extrem example, but it was exactly the piece of code I was looking at, just before I had to read 'Best written'. So I really didn't could bought the 'best written'. Also I know of other times and places where to find such things within the Spring code base. But I never saw something comparable within the Hibernate sources, I had watched.

Do me the favor and have a look at the Hibernate 2.1 source code, the Struts 1.1 source code or even the JDK 1.4 source code. These references are not "excuses"; having a look at those will just give you an indication about the average code quality out there, in particular about the average quality of grown code.

I never reviewed the Hibernate 2.1 code in such an extend. I reviewed the implementation of caching of query statements and the transactional/session related piece of code. I found it equite easy to read. Also the lifecycle threatment was a matter of subject. Maybe I just have to take a more broader look on the Hibernate code. But remember that this comparism might be also a bit unfair. Spring does takle lots of exsting APIs and try to ease their integration. I don't know what you guys went through exactly, but I had to hide quite some APIs behind fascades, so that I might have a little inside knowledge on this topic, too. Remember, before Spring there was a time we had to do this all by ourself.

Ok I will open a little 'review the Hibernate source code' research project. Now I am very nosy about the things you refer to as bing that bad about Hibernate's code-base. If you can give me a little pointer, I would be very pleased.

Also Struts is, what I have never liked. JSP is something that I don't like anymore. In terms of Struts, I simply lack some extense knowledge. I fast read 'Struts in Action' some time ago, but that was it. If I remember correctly Struts looked a way to powerfull and to complicated. But I instantly started to like Velocity at first sight. Struts was something by the way that pointed me to the pre release version of Spring. (did a google search, found a news-group entry and here I am... .)

having a look at those will just give you an indication about the average code quality out there, in particular about the average quality of grown code.

Well there is a statement of Kent Beck which goes something like: whenever you touch code, refactor it. I saw some improvements of the Spring R1.0 to R1.4 in certain areas, but the huge methods got never refactored. Don't you recheck them when finalizing the next release? I don't know your current code-coverage metrics but I would expect that these implementations may provide some lack of coverage (highly guessing). Also talking about average quality (legacy code anyone?) and best written is somewhat diffrent in my point of view that this isn't that related to me.

You know I find that the Spring code is often just hard to read (at least for me). Maybe I am just not used to this particular way of writing. I can not handle a method consisting of more then 100 lines of code. I don't get whats going on there by an easy and straight review session.

The review just don't flow. It slows me down and I don't like being slowed down. Just like I always disliked extense debug sessions and I praise TDD everytime someone talks about a long debug session. I have the feeling that developers talking about extense debug session can be considered as the aquivalent of shark-hunters talking about scares. I don't want scares and I don't want debug sessions. But I don't want hard to read code also. Especially within a framework I use that extensivly (and also promote it extensivly within my friends/colleagues).

In general, you completely neglect the fact that code grows and evolves. The challenges in evolving a large codebase over years are very different from the ones in designing a software system from scratch.

I am sorry, but as far as I know, I had never negelected the fact of code growing and I am aware of the limitations a growing and partly-aging library imposes. I think you are not doing me a favour by stating this phrase without adding workds like 'I guess'. Also I guess, I seam to fail on stateing my suggestions that clearly as I was intended to do. By rechecking my list of suggestions, I don't find much that would break something in terms of backward-compatibility. I also manage an envolving project for four years now. So I had to add some books about version control and the lessons people learned in that field to my bookshelf some time ago. Keeping a library stable isn't that much related to code quality in my point of view. You know using a library is mostly about the architecture and the behaviour of the system. But the architecture isn't much about implementation. It is just about the border between the library and the outside world. This border can be indeed quite long and complecated but inside the library you have much of free hand.

Saidly there are much of the concrete implementations visibile to the outside world and are not placed within a internal subpackage structure to state the instability of these implementations.

So the border is quite long in this case. But refactoring is mostly about two code bases doing exactly the same, having the same sideeffects and flaws but are diffrent in terms of readability - or better speaking - in terms of simplicity (design / implementation).

So I was mostly talking about implementational details. The Spring framework R1.4 consists of about 1000 types and 6000 methods - more exactly speaking 985 types and aprox 5820 methods. I consider around 400 Methods to be a subject of refactoring in mid/long-term and around 50 methods being a subject for shortterm refactoring - around 20 methods I would consider a case for instant refactoring to avoid additional costs. Also I had glaced over much of the methods I consider for short term refactoring and all of the around 20 methods I would consider a case for instant refactoring (beside the one I had fully reviewed).

And so I can truely say, that I have difficulties to easily understand these methods in question and that I am able to exactly pin point the things that make these methods to difficult to read for me.

This is not only about the ubiqitous burden of backwards compatibility; this is also about architectural decisions in a constrained environment.

At most all of my suggestions are non-compatibility breaking refactorings. So I guess you don't do me a favour by calling it all an accademic issue. It is my everyday experience that drive me this way. And I started to fiddle with a KC 85/3, when I was about 7 years old.

It is by no means central to the overall framework, so we don't invest particularly large amounts of time in improving its internal implementation. Of course, it can be improved, no doubts about that - but this is by no means vital for overall Spring.

But the parser is, the BeanWarperImpl is, the PathMatcher is, the bean definition reader is, the autowire bean factory is,.... I tried to understand all of those implementations (expect the autowire factory). And it gave me a hard time. I sometimes just gave in and droped my affords of reviewing those parts and did some sports or mental exercises to gain my self-confidence back. I just feel so little, when I see those complex implementations. I just feel insufficiant when trying to understand those monolites, but I guess I simply arn't used to such extend thinking like I am not used to three hours of bug-hunting anymore.

Also I can not extend something, that I don't fully understand. That's why I am so eager to improve the parser in first place. It is something, I really need to extend and which I can't do easily, like I would consider it should be. So this all isn't purely about the HtmlUtils class.

It was just the one I fought with right before, I had to read the 'best written' ak 'better than everything else' phrase.

I fully agree that this class is a candidate for refactoring. But you seem to assume that we can refactor this whenever we want: This would be true in an academic world. Unfortunately, we can not risk breaking subtle semantics in current behavior; this makes refactoring a major effort.

I know but using just a few 'extract-method' refactorings, a big part of the issues would be solved. That's what I mostly do, when trying to review Spring implementations. Grap a local copy of the code and refactor it by using mainly extract-method and rename fields/methods and to remove documentational lines which always disturb me. It amazes me everytime, how much more readable a monolitical alogrithm implementation gets by simply splitting a huge method into smaller, well named parts. So this is again everything but academic.

...DefaultXmlBeanDefinitionParser... For example, this code is only run on startup, never during operation of an application.

But I already found a gap in the current implementation. Something I would like to suggest being a bug. If I remember correctly, it had something to do with collections and setting up an element of it. There was a not-required attribute without defining a default behaviour if the attribute is missing. It is just a special case with no defined outcome but this is something I would have been expected to be catched by a unit test case.

It's like a bigger web app (around 600 classes/types) I wrote between late 1998- and early 2000.

It ran fine for the next two years (around 100k users per month), no complains and everyone was happy. But I didn't used unit-tests back at those times. For a training session and just to do something usefull during the training, I started to refactor an integrational part, about reading and parsing huge amounts of informations used for synchronizing the representation of the outside world with the current state of the world. I found more then thirty issues, I would consider bugs and I ended up adding just around twice that much objects as the original implementation of that part had before.

The refactoring didn't broke anything. The interface of the modul did not changed, I found some improvements (memory footprint mainly) and the part got's unit tested (boy my attitude towards the reliability of that part changed big times). Also some usable features on the wishlist could be added easily now. And this is something I expect, will happen within some of the Spring framework parts as well. But not in this extreme form (in terms of structure and potential bugs). The code of that part I had refactored/unit-tested was way too old-school and can not be compared the to the code quality of todays Spring framework.

The resulting part was used for additional two years. Now the whole stuff gets replaced by a Spring version, which is not in public offical use today (late alpha state). Most of the classes I wrote back there, could be replaced with 3rd party stuff. But some is still valid after being also refactored the same way.

Let me reiterate that I do by no means object to reworking the implementation of those three classes. My main point is that this is not of incredibly high priority for the overall framework - in contrast to the impression that you leave. These classes might have been the spots that you concentrated on, but this doesn't mean that they are as central to overall Spring as you believe.

I tried to extend the parser to support various ways of contributional support, since I wanted to do a little research on that. I was about to find a preferable way to use contributions without losing the strength of Spring's bean definition mechanism. I always felt that the way to use spring is too architectual strict. I like contributions more then hard-wiring everything using a context.xml.

But to my surprise, I wasn't able to do it and it wasn't lack of knowledge. Even more frustrating, I couldn't understand what's going on, so I had no chance to modifiy the code myself and life with a research version of a Spring code-base. The current parser is hard to read and also non- extendable. 'Best written' does not look that way in my oppinion. Also I have learned that at least one guy tried to understand the same parser story and also finally failed due the same reasons, I did.

Additionally, I consider reading a collection of bean definitions from within a xml file as a core component, because this is what I always do using the Spring framework and reading the reference documentation it seams to be the normal way to go. That a component is just used once at startup, is neither sufficiant nor a necessary test for the component to be unimportant. A problem within this component is a blocker. Without this component neither my unit-tests nor my web application would run. This component is vital for the framework.

Regarding the packaging: Well, as you say, you keep repeating this, but that doesn't make your understanding of Spring's packaging any better. For example, the transaction framework is certainly not J2EE, but arguably the web framework is... your choices are quite arbitrary there.

Packaging based on such coarse categories is hard to get right over the long term, which is why hardly any framework does it.

But the Spring framework has a clear seperation at least for the web part. I am very interested how the Spring RCP sub-project will fit into the spring framework. I would guess that you are not about to add the RCP packages to the main Spring framework project. But I am in doupt.

Also If I have to shape the package structure, I would extract each aspect of the transactional support, which is special to a certain domain. But I just did a test run in order to limit the code base, I use for my reviewing sessions. And it helped a lot. I still push classes and types around so it is really more a prototyping seperation.

About the web vs. j2ee. Web depends on the J2ee components, that is natural I think. But web emerged to be considered a main domain these days. And looking at the size of both resulting subprojects it seams the right way to do it. Maybe considering to name it j2ee.web but this doesn't matter much either. It was truley done in some minutes and got double checked. For me it is a problem to not have a clear seperation of sub-contexts. Event the eclipse people start to extract sub-projects all the time. It has some very strong adds to go for subprojects. In my final exams work (master thesis) I have five sub-projects and the whole project currently consists of only around 250 types (and around 250 test cases). It is a matter of focus and complexitivity control not a matter of size or to JDK or not JDK.

Also extracting sub-projects form within an existing system / library is not breaking any compatibility issues. It is a refactoral decission. But again this is an issue, I would still raise in case some raising his hand and comment a suggestion about doing better and answering 'best written'. I guess this reply is nothing that Alarmnummer find sufficiant for his suggestion.

I also find it quite funny that you complain about too much inline documentation. I believe that such documentation helps a lot in understanding what's going on in the implementation. Javadoc on public methods does not help there at all. In my opinion, the lack of inline documentation is a major issue with Hibernate's source code, for example.

I consider inline documentation as a shift of thinking. And this shift of thinking / context is a point where the flow of reading stops. You have to switch your focus of thinking and that slows you down. Also you have to read natural language instead of code. I once was also a guy of using extense inline documentation, but using unit tests and aiming for small highly focused (responsibility) and well named methods, there is no need for inline documentation anymore.

Even those "//Bug xxxx: foo" documentation lines should become a test-case and nothing to state out in the production code. If I change the code, the unit-tests will fail. So there is no need to do this. The only inline documentation I find acceptable are the "//TODO foo" lines. All the rest should be part of a documentation of a type or public method.

Everything that has to be documentated is more likely to be a written contract, and a contract should be part of a method or type documentation or in case of a private method, a part of a unit-test case.

I have to admit that I think that a hard to read method is just an indication that the writer of this code didn't had all the other developers in mind, who have to review and maintain the particular code-pieces. Readability is no additional sugar, you may aim for, if you want to be called a sweety boy. I consider readability to be the overall prime target. Right after delivering the right solution for a given task (Set of requirements).

>> I find that the Spring code is not agile enough to meet modern coding standards.

Do you really think that your personal view is the single definition for "modern coding standards"?

This is an oppinion, like the 'best written code' is. We can agree or disagree. But today I expect agile software to be easy and cost-effective in terms of usage and maintainance. And doing a lot of reviewing myself and seeing all those figures of the the inter-object dependencies and the long and hard to read methods, including the - in my oppinion - unessary complicated implementions of certain parts, I can still say without getting read, that the current Spring code-base isn't that agile as it should be.

For me agile software is easy to use and easy to maintain. I would consider the code quality of Spring as being truely above average (not to talk about legacy code), but at least some hours of refactoring away from being near to 'most agile' and 'best written'.

I also didn't said that Spring isn't agile. That wasn't my intention. But I say it can be better or in daily experience, it should be better in terms of readablility and extendability.

There are ways to improve it. Most of the improvements should deliver instant use and arn't that costly in my experience. Once I understand the meaning of a method, I can improve the readability within minutes, thanks to the refactoring support of modern IDEs like Eclispe, what I use.

This whole talk remembers me of a problem, I had in conjunction with the transaction manager and the hibernate implementation (back in the old days of Spring RC1 I guess). I tried to check what the hibernate support of Spring is doing in terms of transactions. I failed for around two hard days to figure out, everything I feel is necessary. It wasn't a direct lack of knowledge. I just couldn't read the implementation and did some misassumtion based on what I had read.

After I finished my investigations, I had a sketch of some small sketchy UML/Odell diagramms + some written notes. It should had been by far more simplier. A matter of half an hour. I couldn't reconstruct, what exactly was the problem today, but I guess it was the way transaction
status are optained using ThreadLocals. All the semantic was well hidden and crumbeld within some extense status checking and asserting combined with try and catch stuff. But this was the first but not last time I failed to easily access Spring code of interest.

And do you really think that your bits-and-pieces analysis of classes that you consider important is sufficient for judging the overall Spring code quality?

I did everything but a simple bits-and-pieces analysation of classes that I consider to be important. I have seen the whole spring framework with all the the inter-object dependencies. Please check out the BeanWarperImpl class. It directly depend on 78 types. I just can't imagen
that an implementation depending on 78 types can be considered as being most agile and easy to change/control/extend. I had rarly seen something that much depending on other types to be not a subject for successful simplification. I wouldn't touch that big implementations without reading everything trice, before I make a single assumtion about what's going on if I would change this or that.

I know this is a very delicate topic for you. It is always hard for parents to hear that their kid hasn't proven to be a geniuse by simply putting the right geometrical figures into the right holes on the first try. But I wouldn't be here writing such big posts, if I am not feeling like being a friend of the Spring family. I still use it, I try to contribute improvements and share some thoughts. I am interested in the development and success of Spring.

Also I am still keep reviewing the framework, since by all my difficulties to access some of the most interesting parts, I am still learning something new each time, I review the code. Even if it just keeps me continuesly thinking about how it can be improved and how I would do it and what is better and why.

I invest my time into discovering and extending the Spring framework, because it provides that much of use. But I also think there is room for improvements, even the improvement of core components, so at the bottom line you still can read my judgment to be:

"not 'best written' but well written"
"agile but not agile enough"

So keep up the good work but don't pretend everything is best (ak perfect). Spring does not need that usall marketing speech in my oppinion. Spring delivers true and great value and I don't know much people who took a look at spring and had the power to resist using the framework.


Cheers (and with great respect),

Martin (Kersten)


PS: Of cause, I know how it is to get some critics. Being told that something isn't that good is

always hard. But I hope I could clearify my critics as not being pointless or hip-shot like.

[mschuerig]

Martin,

have you ever considered to apply to your postings (forum and mailing list) what you ask the Spring people to do with their code?

Let me suggest a couple of smells:

• Longwindedness
• Careless composition; not reviewed
• Egocentrism; focussed mostly on the author's opinion
• Allocation of blame; "I may be bad, but so are you"

Also, never tell people their baby is ugly. You may mean it sincerely, but no matter if you're right or wrong -- the people won't like you anymore afterward.

Michael

[klr8]

This is a very entertaining thread indeed!

Maybe we should should also start debating Spring on Linux vs. Spring on Windows and give Slashdot a run for its money!