haruki zaemon

TDD means no writing production code unless there is a breaking test right? Most practitioners agree on this. However the topic of TDD in the context of refactoring came up in a conversation last night. One of my colleagues challenged that because refactoring wasn’t changing the behaviour just the implementation, it can largely be done without a breaking test.

I’m not convinced that is the case. If I think about why we refactor there really is only one possibility - something is wrong with the code. Otherwise, why would we bother. It seems to me this “wrongness” then breaks down into three main causes:

• Missing or superfluous functionality;
• Deprecated implementation; and;
• Smelly code.

The first case is easy to justify writing tests for - clearly, any change in functionality necessitates a test.

The second is more subtle but again, clearly there was some reason for changing the implementation (such as persisting to a database versus in memory) in which case we would need to add some kind of test(s) to verify this.

The third case is a little harder. On what basis do I determine that the code smells and therefore needs a cleanup? In the specific case from last night, Dave had renamed an abstract class to AbstractXXX and extracted an interface. But he had written no extra tests. Admittedly, I usually write a test to ensure that my class implements the necessary interface(s) so he could have done that. However the reason he made the change in the first place was because a checkstyle check was barfing. Why was it barfing? Because it had detected a likely code smell. To my mind, this is a breaking test. It’s a test for code quality as opposed to functionality.

Unfortunately, not all code smells can be picked up by machine (though I’m trying very hard lol). But I thought it was interesting to challenge my own notions of what constitutes a test and, getting back to a previous entry (as is my habit), what kind of test is appropriate in various circumstances.

I also want to see if I can come up with many cases where it is justifiable to modify code without some kind of test to prove that the change is in fact necessary. If I can’t prove that a change is required (and what better way than with a breaking test) why would I make it?

Following on from my entry yesterday on broken tests I came across another interesting problem I see from time to time in peoples test code.

Here’s a very simple example to demonstrate. As far as I’m concerned it’s a big fat broken test:

public class FileBuilderTest extends TestCase {public void testHeaderIsWritten() throws Exception {StringWriter writer = new StringWriter();FileBuilder builder = new FileBuilder(writer);BufferedReader reader = new BufferedReader(new StringReader(writer.toString()));String line = reader.readLine();assertNotNull("too few lines", line);assertEquals(FileBuilder.HEADER, line);...}} public class FileBuilder {public static final String HEADER = "# Created by " + FileBuilder.class.getName();private final Writer _writer;public FileBuilder(Writer writer) throws IOException {if (writer == null) {throw new IllegalArgumentException("writer can't be null");}_writer = writer;writer.write(HEADER);}} 

Seems pretty sensible right? We check to make sure that the header gets written to the stream. Putting aside the are tests for testing or design debate for a moment, one thing remains clear - a test should break if I change the class in anyway that breaks the intended behaviour.

Right?

If so, then this test is truly crippled. Don’t believe me? Well just change the value of header to some absolute garbage and re-run the test.

Did it break?

No. Why? Because the test never checks that what gets written is what is supposed to be written. Instead, it is simply checking that whatever header the FileBuilder has defined is what gets written. So anyone can inadvertently (or deliberately as we have done) change that header and the test never breaks.

I’d rather see this:

public class FileBuilderTest extends TestCase {public void testHeaderIsWritten() throws Exception {StringWriter writer = new StringWriter();FileBuilder builder = new FileBuilder(writer);BufferedReader reader = new BufferedReader(new StringReader(writer.toString()));String line = reader.readLine();assertNotNull("too few lines", line);assertEquals("# Created by " + FileBuilder.class.getName(), line);...}} public class FileBuilder {private static final String HEADER = "# Created by " + FileBuilder.class.getName();private final Writer _writer;public FileBuilder(Writer writer) throws IOException {if (writer == null) {throw new IllegalArgumentException("writer can't be null");}_writer = writer;writer.write(HEADER);}} 

I recall one of the things I found disturbing when reading (one of) Mr. Becks TDD book being his suggestion that our first example is in fact good practice because we are removing duplication. Personally, I don’t care about duplication in tests nearly (if at all) as much as I do in production code.

An interesting variation I heard about involved a developer essentially coding the same algorithm in both the production class and the test so they could verify the results.

At any rate, hopefully you can see the similarity in both cases - the tests weren’t explicit about what the expected results should be instead using a “calculated” value. I’d much rather see the test explicitly state exactly what it expected to be written. That way if someone accidentally does say a find+replace within our FileBuilder code, we will catch it.

I’ve inherited the poorest performing machine on the team. Only fitting I suppose as I’m probably the poorest performing member (it’s a pretty good team!) But the main problem I have is that it takes so long (between 10 and 17 minutes depending on how long it’s been since I rebooted windows) to do a pre-checkin build (to make sure I haven’t screwed anything up) that by the time it finishes, I really need to to another CVS update. This invariably brings back a number of changes that good old paranoid me thinks necessitates another local build. This vicious cycle continues until I give up and just check in my code.

Of course I then need to wait until the integration build on the cruise machine completes before I can be happy I’ve not broken anything. Now the chances are that there is already a build in progress and, on average, about half-way through. The integration build itself takes about 20 minutes all up (and that’s on a fast machine). So, it’s now about an hour by the time I’ve got anything checked in and I’m happy I can go home.

This morning, Andy Trigg removed the generation of the test reports from the local machine build as this was taking around 2-3 minutes on my machine and besides, it’s really only needed if the tests break right?

But that didn’t really help all that much and I finally cracked it and decided to take a look at what was causing the build to take so long. How long can it take to compile and run 1200 unit tests and around 250 functional tests? I mean sheesh! A lot of time and efforts has gone into ensuring that the unit tests are as close to “real” unit tests as possible and not just functional tests in disguise so they should be blindingly fast to run.

Closer examination of the build file revealed that the tests were being forked. This was because there is some bug in the build process that causes OutOfMemoryErrors. This was clearly going to slow things down but not much I can do about it as far as quick wins goes.

The next thing I noticed was that version 1.6 of ant has a new option on the JUnit task (reloading) that allows you to confugure if a new class loader will be used for running each test class. I quickly set this to false!

And lastly, I discovered that if I ran all the unit tests in IntelliJ they took around 30 seconds but run from the Ant script they take around 3 minutes!

Then I remembered playing around with test suites some time ago. I vagualy recalled that putting tests into a suite ran orders of magnitude faster than if you ran them one at a time. A few minutes using good old of find, sed, grep, etc. and I created a class named Suite that looked something like this:

public class Suite extends TestCase {
    public static Test suite() {
        TestSuite suite = new TestSuite();
        suite.addTestSuite(BlahBlahTest.class);
        suite.addTestSuite(FooBarTest.class);
        ...
        return suite;
    }
}

Then I changed the build script to run my suite instead of each individual file, making sure that I forked it just in case.

Well blow me down. The pre-checkin-build now runs in 3 1/2 minutes! That’s a full clean and rebuild plus unit and acceptances tests. Whats more amazing is the actual unit tests now take an amazing 9.4 seconds to run. A clean unit test build now only takes 1 minute and an incremental build a mere 30 seconds. Woohooo! I can’t wait to see how the cruise box performs now as it was already at least twice as fast as my machine.

The only real issue was then the fact that we would have to maintain the suite to ensure we didn’t leave out any tests. Stuff that I thought so I quickly whipped up an ant task to generate it for me and away we went. I also suggested we could remove the need to compile the suite by generating the byte-code directly to which James just frowned dissaprovingly and accused me of being “such a geek!” Maybe he’s right? hehehe.

Next week I’ll see if I can get similar improvements in the acceptance and regression tests.

Oh yeah the test report generation now takes a whole 1.5 seconds to run so it might make it’s way back into the local build again but I doubt it. Every second counts! :-)

I’ve been doing some incidental code cleaning as I implement functionaility for the system I’m currently working on. It’s something I really enjoy. Code quality is somewhat of an obsession. I often wonder if it’s actually worth it. But I dislike leaving “ugly” code lying around so I fix it anyway. I’m sure some of the developers wished I’d just shut up. Some of them have even taken to not letting me actually look at the code when answering a problem for fear that I’ll get distracted :-)

Last year, James Ross and I wrote some 20+ custom checkstyle checks (most have made it into the core distribution). They were brought about by a project we were both working on at the time. We would manually identify code smells and then set about developing some heuristics for automagically finding all of them.

I’ve been applying most of those same checks to this project and, I’m happy to say, although they turned up some “violations”, for the most part the code base is pretty clean. I still come up with more checks I’d like to implement when I decide to make the time and last night I made the time and wrote a very simple check for “missing abstractions” or more properly, opportunities to normalise.

There are clearly many ways we determine that there are missing abstractions but there is one thing I’ve noticed on many projects: code containing common prefixes and/or suffixes is often a good candidate for extracting a class. Some very simple yet recurring examples might be date ranges (dateFrom and dateTo) and addresses (addressStreet, addressCity, `addressPost``), etc. I’m sure you can think of other less obvious examples.

The check simply identifies clusters of instance variables or parameters that share a common prefix and/or suffix. When I ran it against the code base this morning I was amazed to find hundreds of examples! Probably 1/3rd were in constants: MINIMUM_AGE, MAXIMUM_AGE, etc. with the remainder largely instance variables and a small number of parameters.

After trawling through the results, I’d say I had probably found a dozen or more absolute, no-bones-about-it cases for refactoring which I thought was a pretty good result. What concerned me was there seemed to be a lot of noise. But then again, was it really noise? Would it not be better to have minimum and maximum values actually represented as a Range? If so, how far do you perform this kind of normalisation? As James pointed out, humans really do have a left and a right arm. Do we really need to make an Arms class with getLeft() and getRight()? Probably not. It could be argued though that constants sharing the same prefix/suffix such as _ACTION_NAME or DISPATCH_ are really just introducing an implicit namespace that might be better moved to a separate, explicit, enumerated type?

I’d be interested to know what you think and if you’d be interested in running the check against your own code to generate some feedback. To do so, download the jar file and add it to your build class path. Then, to run the check, simply add the following code fragment to your checkstyle configuration as a child of TreeWalker:

<module name="au.com.redhillconsulting.jamaica.tools.checkstyle.MissingAbstractionCheck"/>

You’ll also find some more checks (all in the same package) that have yet to make it into the checkstyle distribution:

• ClassDataAbstractionCouplingCheck
• ClassFanOutComplexityCheck
• DuplicateLiteralCheck
• FinalFieldCheck
• GuardingIfCheck
• NPathComplexityCheck
• NullPointerCheck
• ReturnFromCatchCheck
• ReturnFromFinallyCheck

As I mentioned, I’ve been doing some code cleaning up as I add new functionality to the system I’m currently working on. I have to agree with Andy Trigg and say the delete refactor is one of my favourites. It’s gratifying to replace great swathes of code with something much simpler and yet more functional. The best is when I refactor some code that allows me to delete a test. This is usually because responsibility for some behaviour has been smushed over a number of classes which leads to duplication among several test classes.

If it weren’t for all those failing tests, I’m sure I would have screwed up pretty early on. As I was fixing tests here and there, I noticed something interesting. Some of the tests were breaking due to assertion failures. Just what you would hope for. However, rather more were failing due to exceptions. Usually a NullPointerException. What’s worse is the NPEs were in the test case code, not code the test was calling.

Tests should assert expected behaviour. If you expect a return value not to be null, be explicit about it. If you expect an iterator to have more values, assert it. Don’t wait for the underlying code to blow up in your face unless you’re actually testing for that behaviour of course.

I also like to keep tests as small, simple and obvious as possible. If a test case checks for a limited set of conditions, its much easier to work out where to fix the code when it breaks. Heavily factored tests may appear small, but if you were to inline all the test code, you’d find that it’s definitely not simple nor obvious.

What I’ve found most interesting is that the worst tests are generally the ones written after the fact. The “oh yeah I’m gonna need to test this” kind. They stand out like the proverbial k9’s gonads. They’re usually long (the tests not the testes), difficult to follow, test more than just a few simple concepts and tend to be very brittle. That last point seems counter intuitive at first but is definitely backed up by experience.

So what’s the point? Tests aren’t just for keeping jCoverage happy! In fact crappy tests are a liability. They will end up being a maintence nightmare if left unchecked (pardon the pun).

I was sticking my nose into a code review James Ross was performing the other day. Among other things, naturally, he was checking for code duplication. Now my informal analysis of open source projects and commercial projects I have been brought onto suggests that, a figure of anywhere between 10% and 20% code duplication is pretty standard. As horrifying as that may seem (to me at least), it was rather amusing to think that the final report might include words to the effect that, “your software falls within industry norms with respect to duplication of source code.”

This lead me to question what is acceptable. Why is it that one developers clean code is anothers quagmire? James suggested that over and above the obvious differences in experience and “smarts”, it may come down to how much “smell” we can tolerate. He likened it to cleaning the house - Some people (ok me!) can leave dishes in the kitchen for days while others (like James) apparently can’t go to bed if there’s even one dirty dish lying around.

James also recalled an article he had read on Broken window syndrome and we wondered if the same thing applied to software via accumulated technical debt? That maybe as the density of code smell increases, there is some point beyond which there is no escaping. A point beyond which your team subconciously stops caring anymore because “well the rest of the code is crap so I’ll just add this hack in here.”

Maybe that’s why Windows™ is so very broken? ;-)

I recall my mum being infuriated that as kids, we would simply walk past the rubbish or toys on the floor as if they weren’t there. She couldn’t understand why it never ocurred to us to pick up the Lego and put it away. Even now I find myself noticing a piece of paper on the floor and forcing myself not to just leave it.

So while your team might start to understand the value of design patterns, TDD, whatever, it’s really important to get them to be more sensitive to identifying and eliminating rubbish as they walk through the code and not just leave it there for mum to pick up.

Your business analysts have no doubt spent days coming up with test cases for an interest calculation. Writing it all out in nice, simple, plain english so you can understand what you need to implement. They’ve probably produced half-a-dozen scenarios for you to test against. Don’t go and spoil it all by naming your method testInterestCalculation. I want to see testInterestCalculationWithFixedRateFor24Months or something similar.

I know there will be those who say that it’s redundant information to encode into the test name. I mean just look at the code right? Phooey! Ever written a method named remove that actually added something instead? If the answer is yes, go directly to jail. Do not pass go. Do not collection a hundred dollars! If, like most sane people, the answer is no, that’s because we name methods to indicate intention. We then look at the code to see how it’s been implemented.

Ever spent what seemed like hours trying to come up with a good name for your class? Why bother? Again, because it’s so important to communicate not only to others but also to yourself the role of the class, it’s purpose for being. Without an appropriate name, it’s often hard to know what the classes responsibilities should be.

If I go onto a project and need to get an idea of what the system does, the first place I look is the test suite. It should tell me almost everything I need to know about the implementation.

So do me a favour and spend a bit of time naming your test cases something meaningful. Please! I beg you. It’ll stop me wasting your time asking lots of stupid questions ;-)

Jon - Six Million Dollar Man - Eaves raises some interesting points on spiking versus TDD. A very good summary on a tried and true approach for learning new stuff. While I agree totally with the article (I do this myself quite often so it must be good hehehe), I’ve really started to try and use JUnit as a mechanism for doing spikes as well.

Just recently, as part of a new 3-month gig (many thanks to David Pattinson and James Ross and much to the obvious displeasure of one who shall remain nameless) I started doing some investigation into ILog JRules, something I had never used before. My primary responsibility was to mitigate any technical risk associated with using JRules. Naturally, testability was one of the main concerns for the team.

Now usually in this kind of scenario, one would usually whip up a quick-and-dirty main method, and start writing some code to both learn about JRules and to prove what it can/can’t do. Instead, I started off by writing a test. Of course I had no idea what the test would do but I wanted to force myself to do it anyway.

At first it was a very simple test to work out what jar files I would need to just get the engine up and running. Then as I had more and more dicsussions with various team members about what our overall requirements would be and we thrashed out the different ways we could achieve the same thing with JRules, I simply added more and more sophisticated tests to test out our assumptions.

Doing my spiking as fully-blown tests, forced me to think much more critically about what I was doing rather than just spew stuff to stdout and look at the results. I was forced to clearly state my assumptions and the tests proved conclusively if these assumptions were right or wrong.

The great thing now is that we have a full compliment of integration tests that both prove our assumptions about, and demonstrate our chosen use of, JRules. This will be extremely valuable when the rest of the team start to write their own rules as part of implementing user stories. It also means we can happily upgrade to newer versions of JRules if/when they are made available with much less worry about breaking our code. This is of course why you all write integration tests isn’t it ;-)

A few days ago, James and I did much the same thing when doing some swing development. I can’t say I’ve perfected it. I still do some debugging and println but in the context of writing a test. But, I’m forcing myself to be as “pure” about it as I can to see how far it’s practical to push the technique.

As a complete aside, I found JRules pretty damn easy to use and test. The documentation is very good. If anyone is interested, I’ll post a brief overview.

As requested, my brief overview of JRules. I’m sure I will have failed to answer many questions so feel free to ask.If I know the answer, I’ll add it to the entry, othwerwise I’ll either cop out and say I don’t know or go and find out.As always, if I’ve made a mistake or three, feel free to point them out.

First off, I found JRules very easy to use. It comes with very good documentation. As I mentioned in aprevious entry, foremost in my mind was to prove testability. I can safely say that rules can be fullyunit tested. So there are no excuses! ;-)

Like many rule engines, JRules usess a Rete network. The cool thing about the Rete algorithm is that rules with similar conditions (or subsets of conditions) are identified and evaluated together meaning that theconditions need only be tested the minimum number of times. This differs from java `d rules where typically everycondition in every rule must be evaluated regardless of whether we might already have tested for it in previous rule.

The centre (no, this is not a spelling mistake) of the programmatic world in JRules is the rule engine (IlrContext) also known as a context. Thecontext holds a set (IlrRuleset) of rules (IlrRule). The context also holds facts. Facts are simplejava objects and, as the name implies, are the things we know about the state of the processing “world”. As you add(assert) facts about the state of your “world”, the rule engine updates the agenda. At any given point,the agenda holds the rules that have conditions matching the current state and are therefore ready for firing.

Once a rule has fired, it is removed from the agenda and will not be made available for firing unless it, or anotherrule, modify the current state such that the rule conditions would once again be satisfied. This is pretty neat as itmeans you can have rules assert facts that cause other rules to fire.

Rules can be assigned a priority to assist in determining the firing order. The specific ordering rules are clearlydocumented so I wont repeat them here.

Rules can be grouped into “packets”. Packets can then be referenced (and executed) by name. This can be very usefulif you have certain classes of rules that you know you will/wont need during processing and therefore wish toprogrammaticaly enable/disable at runtime. JRules 4.5 also provides a RuleFlow mechanism which is supposedlythe “preferred” method of achieving the same thing. I must admit that in our case, ruleflows didn’t seem appropriate so we have so far stuck with packets.

There is one more facility for filtering rules from the agenda. These firing filters are applied to rules whosconditions have already been evaluated and are in the agenda. This differs from packets and ruleflows which selectivelyadd/remove rules prior to evaluation.

Rulesets support in, out and inout variables which are then available to all rules withinthe ruleset. This is one mechanism you could use to parameterise your rules.

JRules out-of-the-box provides three languages in which rules can be written:

• IRL - ILog Rule Language;
• TRL - Technical Rule Language; and;
• BAL - Business Action (Natural) Language.

I started off using IRL as it was the language given in the documentation examples. It’s a fairly low-level language.I think from a geeks perspective it’s the one to start with as it gives you a good understanding of what actually goeson. IRL can be written using the text editor of your choice. A simple example of IRL might look like:

when {?order: Order(product == Product.CHAMPAGNE);Customer(state == State.VIC; status == CustomerStatus.GOLD) from ?order;} then {?order.applyDiscountPercentage(15.00);System.out.println("Discount applied!");}

This essentially says, whenever an order for champagne is raised by gold customers in victoria, apply a 15% discountand print a message to that effect.

Each condition in the when clause is evaluated in the order declared. The conditions are AND’ed together.If the condition(s) are met, any statements in the then clause are executed when the rule is fired. Forcompleteness, there is also an else clause.

As you can probably see, the IRL is a Java-like language supporting, as far as I can tell, almost all usual javasyntax (including imports) plus the additional syntax required for describing rule conditions.

TRL is pretty much the same as IRL but is more verbose. For example:

when there exists an Order(product == Product.CHAMPAGNE)...

Strictly speaking, the BAL is actually an example language that demonstrates how you can buildyour own language on top of the underlying languages. Having said that, the BAL is almost feature complete and manybusinesses will choose it over the TRL and IRL. The intention is that business people can write in BAL. In actualality, I see this being about as likely to happen asbusiness people using Crystal Reports to write custom reports. If you can make it happen, great but I very much doubtit. However, BAL is much easier to read especially for business people and even for developers. So when, as a developer,you need to validate that the rule you’ve just written is correct, BAL is a god send. Even mildly complex rules arequite hard to follow in raw IRL and, for that matter, TRL.

BAL also requires a mapping layer between your Java objects and natural language. This isso that instead of writing order.getTotal() we can instead use the total amount of the order. On anagile project that is ramping up, I can see this as being an small but certainly not insurmountable issue due to theever changing domain model.

JRules comes with a GUI Editor that allows you to maintain rules in any of the supported languages. While it ispossible to create rules by hand in IRL and stored as plain text files, TRL and BAL require the GUI. The rules arestored in a repository (directory structures) that can then be exported to an IRL file for use by an application. Again,Not so much of a problem really but being forced to point and click when I really just want to type is very frustrating.If I could ask for one thing, it would be the ability to edit BAL rules in my editor of choice and then import them intothe repository.

BAL and TRL are both converted to IRL. IRL can be converted back into TRL. Unfortunately theconversion from BAL to IRL is a one-way street. Taking a look at the generated IRL was quite an eye-opener. The ruleeditor converts BAL into IRL that is almost impossible to understand by a human.

From a purely technical perspective, it matters little what language the rules are written in as they end up as IRLwhich is then parsed and executed by your application. Loading and running a test requires only one jar file(jrules-engine.jar) and can be as simple as:

public void testEngineIntegration() {// Define a simple rulefinal String rule = "rule TestRule\n" +"{\n" +"    when\n" +"    {\n" +"        not String();\n" +"    }\n" +"    then\n" +"    {\n" +"        assert Integer(57);\n" +"    }\n" +"};";// Parse the ruleIlrRuleset ruleset = new IlrRuleset();assertTrue("Rule parsing failed", ruleset.parseReader(new StringReader(rule)));// Create a context (rule engine)IlrContext context = new IlrContext(ruleset);// Execute the rule and ensure it was actually runassertEquals(1, context.fireAllRules());// Check the final state matches the expected stateEnumeration enum = context.enumerateObjects();assertNotNull(enum);assertTrue(enum.hasMoreElements());assertEquals(57, ((Integer) enum.nextElement()).intValue());assertFalse(enum.hasMoreElements());}

You can also load rules directly from the repository if you wish. So far we haven’t attempted this and for the timebeing at least it’s not a high priority as we will be loading the ruleset from the exported script.

The repository, apart from being a pain to use, has some other mildly annoying issues when working in an agile team.These are mainly to do with locking rules for editing. The rule editor allows you to create rule packages (not to beconfused with packets) and it is only at these levels that locking is possible. Again, not too much of an issue.Besides, you will quickly find your rules unmanageable if you don’t group them at all. Packages are also useful if/whenwriting ruleflows. Remember however that these packages are purely logical and affect only the names of the generatedrules. Something you will most likely rarely have to worry about anyway.

Generally, I would recommend keeping your rules as simple as possible. If your find your rules blowing out to four pages, stop and think again. Rules can and should be decomposed into smaller rules. James Ross has been reading a great book on the subject, Principles of the Business Rule Approach, which he says is a must read for anyone embarking on business rules analysis.

People who know me well, know well (and it’s not even christmas) my disdain for frameworks in general. I must have missed it but at some point the ideal of component based development was hijaked by the framework. Was this because it didn’t actually work or because someone decided to play a cruel joke on us to see how long we, as an industry, could keep going around in circles re-inventing the proverbial wheel?

Almost every large-scale project I’ve ever seen starts of with lofty goals of re-using the framework dejour and soon realises that it just wont cut it. Then comes the process of deciding if it is better to stick with it and forego the application you really want/need, roll-your own for maximum flexibilty or bastardise it so as to render your application largely incompatible with any future releases of said framework.

Sourceforge is littered with every-man-and-his-dogs attempt to build a better mousetrap. And they’re just the open sourced ones. I dread to think how many companies have their own flavours!

I’ve had much success building applications from scratch with no framework, just some user stories and test cases. Then letting the infrastructure requirements emerge and hunting around for libraries that will satisfy my needs. This seems to work much better than the times I’ve tried to decide up front which frameworks and tools I’d like to use, invariably leading to more work than it was worth. The key point here is identifying real requirements and addressing them. “We will need struts” is not a requirement!

Off the top of my head, I can think of maybe half-a-dozen libraries that I’ve been using regularly that don’t lock me into someone elses idea of how my application should be structured. Some for persistence, some for rendering, others for business rules, etc.

Sometimes there are so-called “strategic” reasons for using a particular tool. Unfortunately, the average whiteboard architect has usually been drinking way too much Gartner Koolaide. To be re-usable, software components should solve a particular problem without trying to dictate my overall design. And more importantly, they should solve the problem well.

Yikes! Is that bile I can smell? Hmmmm

TADAIMA!

It’s been sometime since my last entry (feels like the start of a confession) but I’ve been in Japan for the past 2 weeks. For various reasons I really needed to get away from computers for a bit (no pun intended) and what better way than to emerse myself in my other passion.

As I was sitting in the plane on the 9 hour leg from Sydney to Tokyo, I was thinking about genetic programming (GP) and good old fashioned artificial intelligence (AI). I remember being fascinated by AI when I was a kid only to discover that in essence it was all about fitting a curve to a line. In more recent years, I stumbled on GP which sparked my interest once again especially when I read about people taking out patents on the basis of genetic algorithms (GA) they had developed specifically for the purpose of producing patentable designs.

So anyway, I started to think back to simple (and not so simple) neural nets that I had worked with. The kind where you create a back propogating net, feed it the expected inputs and outputs and watch it reconfigure the weights, etc. accordingly.

This got me to thinking about my old friend test driven development (TDD). Neural nets “evolve” by reconfiguring themselves to satisfy the old requirements AND the new requirements. TDD encourages a similar behaviour.

One of the criticisms of neural nets is they can be quite brittle. That is, they may very well solve the problems they have trained for but can often fail dismally on problems that to us seem almost exactly the same yet obviously differ enough to “confuse” the net. Usually it’s just a matter of re-training the ’net with all the old data AND the new data (just like running your tests!). The trick is to understand the problem sufficiently to create useful training data.

One of the criticisims of TDD has been that it creates brittle applications. Now my personal experience is that is just rubbish. If anything, it encourages designs that are more flexible/extensible in the long run. But, it is true to say that a purely TDD application may not cope with a scenario that hasn’t been tested for. In fact I argue that anything that hasn’t been tested for is by definition not a feature of the system. Not only is it not a feature of the system now, it is definitely not guranteed to work in any future release even if it does work by coincidence now.

But the real thing I liked about comparing TDD with AI is that it’s evolutionary. The system is continually adapted to suit the changing needs. This naturally lead me to think about GP and wondered if it was possible or even practical (let alone useful) to write a GA that could take a suite of failing JUnit tests and generate an application that would make the tests pass? Then we would simply add some more tests and re-run the GA to produce a new system adapted to the new requirements.

As I’ve already stated, I’m not sure how practical this is let alone useful even if it is possible. But it was the last thought on computers I had until I arrived back in Melbourne.

Now to find a good Japanese restaurant…

I recently added <a href=“http://www.joverage.org”’>jCoverage to the build of a “mostly” TDD project. The bits that weren’t TDD are classes that we either “hacked” together for a quick-and-dirty proto-type (and that will ultimately be removed) and some that we used for domain modelling.

I like test coverage results, mainly because it gives me a warm fuzzy feeling. I can see how well or how poorly the developers are going WRT test coverage. But, I’ve often wondered if it has much benefit over and above the warm fuzzies.

It has occured to me many times that in many cases (especially on a TDD project) the coverage analysis really shows me potential areas of dead code. That is, code that almost never (if at all) gets executed. Especially on large projects with lots of re-factoring going on, it can be hard to keep track of which methods are no longer used.

IDEs such as IntelliJ IDEA and Eclipse and even Checkstyle and I think PMD can show you visually which private members aren’t being used. The IDEs can also show you which public and protected methods aren’t being used but you have to run the search manually. Granted, it is also possible to write some code that loads all your classes and builds dependency graphs to do the same. But why bother when I already have a tool that does it for me?

And so it came to pass that this morning I was looking over a coverage report and found a few areas where the coverage was a big fat zero. Intrigued, I opened the code in my IDE and did a search for all usages. What do you know. None. Zero. Nada. Bupkis. Zilch. You get the idea :-). I did this on the next few untested methods and I’d say that around 25% of the untested code was actually dead code!

Just for curiosity, I’d really like to put this into a live system and see which parts of the code aren’t used. I’m sure some of it will turn out to be old code that handles scenarios that never eventuate anymore. Who knows? Worth a try at least?

Not much this time except to say that I took the previous examples and made them a bit more generic. The example provided shows the simplest method of using the classes but it can easily be extended for more complex requirements. In fact, I’ve so far used these classes to successfully test some in-memory database code I’d been writing so it definitely works for other than tivial examples.

/*** Based on article http://www.npac.syr.edu/projects/cps615fall95/students/jgyip5/public_html/cps616/conflict.html*/public final class SimpleSample {private int _common;/*** Increment the common variable* @return true if we managed to update it "atomically", otherwise false to indicate failure*/public synchronized boolean increment() {int common = _common;Thread.yield();boolean successfull = (_common == common);_common = common + 1;return successfull;}}

import java.util.LinkedList;import java.util.List;public class SimpleSampleTest extends CustomTestCase {public SimpleSampleTest() {super(SimpleSample.class);}public void test() {final SimpleSample sample = new SimpleSample();List targets = new LinkedList();for (int i = 0; i < 5; ++i) {targets.add(new CustomRunnable() {public boolean run() {return sample.increment();}});}execute(targets);}}

import org.objectweb.asm.Attribute;import org.objectweb.asm.ClassAdapter;import org.objectweb.asm.ClassVisitor;import org.objectweb.asm.CodeVisitor;import org.objectweb.asm.Constants;/*** Removes synchronization from a method. Currently only removes the `synchronized` access flag from the* method declaration.*/final class ClassModifier extends ClassAdapter {public ClassModifier(ClassVisitor visitor) {super(visitor);}public CodeVisitor visitMethod(int access, String name, String desc, String[] exceptions, Attribute attributes) {int newAccess = access;if ((access & Constants.ACC_SYNCHRONIZED) != 0) {newAccess -= Constants.ACC_SYNCHRONIZED;}return super.visitMethod(newAccess, name, desc, exceptions, attributes);}}

import org.objectweb.asm.ClassReader;import org.objectweb.asm.ClassVisitor;import org.objectweb.asm.ClassWriter;import java.io.IOException;/*** Forces certain classes to be loaded into this class loader. In addition, performs byte-code modification to remove* synchronization from specific classes.*/final class CustomClassLoader extends ClassLoader {/** Name of the class under test */private final String _subjectClassName;/** Name of the test class */private final String _testClassName;/*** Constructor.* @param subjectClassName Name of the class under test* @param testClassName Name of the test class*/public CustomClassLoader(String subjectClassName, String testClassName) {_subjectClassName = subjectClassName;_testClassName = testClassName;}public synchronized Class loadClass(String name) throws ClassNotFoundException {Class c = null;if (name.startsWith(_subjectClassName)) {c = defineClass(name, true);} else if (name.startsWith(_testClassName)) {c = defineClass(name, false);} else {c = super.loadClass(name);}return c;}/*** Forces the loading of a class into this class loader* @param name The fully qualified class name* @param modify* @return The newly defined class* @throws ClassNotFoundException If an error ocurrs during loading*/private Class defineClass(String name, boolean modify) throws ClassNotFoundException {// Setup the class file to readClassReader reader = null;try {reader = new ClassReader(getResourceAsStream(name.replace('.', '/') + ".class"));} catch (IOException e) {throw new ClassNotFoundException(name, e);}// Setup an in-memory writer for the byte-`ClassWriter writer = new ClassWriter(false);// Determine if we need to modify the classClassVisitor visitor = writer;if (modify) {visitor = new ClassModifier(writer);}// And load itreader.accept(visitor, false);byte[] byteCode = writer.toByteArray();return defineClass(name, byteCode, 0, byteCode.length);}}

/*** Convenience interface for constructing simple threaded tests.* @see CustomTestCase#execute(java.util.List)*/public interface CustomRunnable {/*** Performs the test.* @return true to indicate success, otherwise false to indicate failure*/public boolean run();}

import junit.framework.TestCase;import junit.framework.TestResult;import junit.framework.TestSuite;import java.util.Iterator;import java.util.LinkedList;import java.util.List;/*** Base class for testing thread safety. Extend this class and simply implement any tests you need. Each test will be* run once ASIS and then again with synchronisation removed to, hopefylly, induce failure. When run with synchrisation* remove, each test is checked to ensure that it failed.* <br>* There is also a convenience method `execute(List)` to assist in writing simple tests.* @see #execute(java.util.List)*/public class CustomTestCase extends TestCase {/** Names of classes to modify */private final String _subjectClassName;/*** Constructor.* @param subjectClass Names of class under test*/public CustomTestCase(Class subjectClass) {_subjectClassName = subjectClass.getName();}/*** As the name implies, re-runs all tests (except itself) with synchronisation removed.*/public final void testAllUnsynchronized() throws Exception {// Ignore recursive invocations of this particualar testif (getClass().getClassLoader() instanceof CustomClassLoader) {return;}// We'll need our custom class loader to perform byte-code manipulationCustomClassLoader loader = new CustomClassLoader(_subjectClassName, getClass().getName());// We need an instance of this test within the new class loaderTestSuite suite = new TestSuite(loader.loadClass(getClass().getName()));// Run the testsTestResult result = new TestResult();suite.run(result);// And and ensure they ALL failed// TODO: This is not sufficient for reporting purposes. We need to check and report on each testassertFalse(result.wasSuccessful());}/*** Convenience method if a simple threaded model is all you require. This method executes the specified* `CustomRunnable`s and asserts that each thread completed successfully.* @param targets Collection of `CustomRunnable`s to execute in parallells*/protected final void execute(List targets) {// All threads are to share the same thread groupfinal ThreadGroup group = new ThreadGroup(getName());// Create a bunch of threads, one for each targetfinal List threads = new LinkedList();for (Iterator i = targets.iterator(); i.hasNext(); ) {threads.add(new CustomThread(group, (CustomRunnable) i.next()));}// Start up the threadsfor (Iterator i = threads.iterator(); i.hasNext();) {((CustomThread) i.next()).start();}// Wait for them to finish and determine if they were all successful or notboolean successful = true;for (Iterator i = threads.iterator(); i.hasNext();) {CustomThread thread = (CustomThread) i.next();try {thread.join();} catch (InterruptedException e) {// Ignore it}successful &= thread.wasSuccessful();}assertTrue(successful);}}

/*** Executes a `CustomRunnable` and reports on the success or failure.* @see CustomRunnable*/final class CustomThread extends Thread {private final CustomRunnable _target;private boolean _successful = false;public CustomThread(ThreadGroup group, CustomRunnable target) {super(group, "");_target = target;}public void run() {try {_successful = _target.run();} finally {if (!_successful) {// Fail-fast - interrupts all sleeping threadsThread.currentThread().getThreadGroup().interrupt();}}}public boolean wasSuccessful() {return _successful;}}

At a loose end today I turned my attention back to a recent blog of mine on testing for thread safety. Spurred on by your feedback and possibly just to prove a point ;-), I decided I’d spend a few hours and see what I could come up with.

Before delving into the code, I’ll set out the scope or terms of reference for the excercise:

• I wanted to test a very simple class for thread-safety;
• The class should be written (designed) with thread-safety in mind, ie. with synchronization in place;
• I will only deal with synchronization at the method delcaration level
• The tests should prove that the class works correctly with synchronization in place;
• The tests should also prove that the class fails when sychronization is removed; and finally;
• I want it to be automated so I need a way to have a synchronized and unsychronized version of the class.

The last point here could be handled by using an interface much like the synchronized collection wrappers but I felt this was kind of cheating. Instead I decided to use the ASM byte-code library to do some magic on the class files.

Now onto the code and some brief explanation of the classes. You should be able to copy and paste the code into your favourite IDE, compile and run. You’ll obviously need JUnit and the ASM byte-code library.

First the class under test which I based on some code I’d seen when researching threading:

import java.util.Random;

public final class ThreadSafe {
    private int _common;

    /**
     * Increment the common variable
     * @return true if we managed to update it "atomically", otherwise false to indicate failure
     */
    public synchronized boolean increment() {
        int common = _common;
        spin();
        boolean successfull = (_common == common);
        _common = common + 1;
        return successfull;
    }

    private void spin() {
        Random random = new Random();
        try {
            Thread.sleep(random.nextInt(500));
        } catch (InterruptedException e) {
            // Ignore it
        }
    }
}

As you can see, very very simple. Its sole reason for existing is to demonstrate how multi-threading can cause conflicts. The class is correctly synchronized. That is, if left unmodified it should behave correctly and with no failures in a mutli-threaded environment. However, if multiple threads were to execute through a single unsychronized instance, we would hopefully end up with a failure. The call to sleep() with a random duration is a means to that end.

A slight variation on this would be to update an unsyhronized Collection instead of a simple counter. The Collection classes typically throw a ConcurrentModificationException which we could either catch and return false or modify our test to catch the exception itself.

Next, the somewhat large test class which is broken up into a few inner classes for simplicity. Don’t believe that it’s simpler this way? Try splitting them out and see what happens. It’s possible, but some of the logic becomes too complex for my liking.

Anyway back to the code:

import junit.framework.TestCase;import org.objectweb.asm.Attribute;import org.objectweb.asm.ClassAdapter;import org.objectweb.asm.ClassReader;import org.objectweb.asm.ClassVisitor;import org.objectweb.asm.ClassWriter;import org.objectweb.asm.CodeVisitor;import org.objectweb.asm.Constants;import java.io.IOException;import java.lang.reflect.Method;import java.util.Iterator;import java.util.LinkedList;import java.util.List;public class ThreadSafeTest extends TestCase {public ThreadSafeTest(String name) {super(name);}public final void testSucceedsAsis() {assertTrue(execute(getName()));}public final void testFailsWithoutSynchronization() throws Exception {// We'll need our custom class loader to perform byte-code manipulationCustomClassLoader loader = new CustomClassLoader();// We need an instance of this test within the new class loaderClass testClass = loader.loadClass(getClass().getName());// Run itMethod method = testClass.getMethod("execute", new Class[] {String.class});assertFalse(((Boolean) method.invoke(null, new Object[] {getName()})).booleanValue());}/*** Runs the actual test.* @param name The name of the test* @return true on success, otherwise false to indicate failure*/public static boolean execute(String name) {// The class under testfinal ThreadSafe target = new ThreadSafe();// All threads are to share the same thread groupfinal ThreadGroup group = new ThreadGroup(name);// Create a bunch of threadsfinal List threads = new LinkedList();for (int i = 0; i < 5; ++i) {threads.add(new CustomThread(group, target));}// Start up the threadsfor (Iterator i = threads.iterator(); i.hasNext();) {((CustomThread) i.next()).start();}// Wait for them to finish and determine if we were successfull or notboolean successfull = true;for (Iterator i = threads.iterator(); i.hasNext();) {CustomThread thread = (CustomThread) i.next();try {thread.join();} catch (InterruptedException e) {// Ignore it}successfull &= thread.successfull();}return successfull;}/*** Runs the class under test and reports on the success or failure.*/private static final class CustomThread extends Thread {private final ThreadSafe _target;private boolean _successfull = false;public CustomThread(ThreadGroup group, ThreadSafe target) {super(group, "");_target = target;}public void run() {try {_successfull = _target.increment();} finally {if (!successfull()) {// Fail-fast - interrupts all sleeping threadsThread.currentThread().getThreadGroup().interrupt();}}}public boolean successfull() {return _successfull;}}/*** Performs byte-code modification to remove synchronization from the class under test.*/private static final class CustomClassLoader extends ClassLoader {public Class loadClass(String name) throws ClassNotFoundException {if (name.equals(ThreadSafe.class.getName())|| name.startsWith(ThreadSafeTest.class.getName())) {return defineClass(name);} else {return super.loadClass(name);}}private Class defineClass(String name) throws ClassNotFoundException {// Setup the class file to readClassReader reader = null;try {reader = new ClassReader(getResourceAsStream(name.replace('.', '/') + ".class"));} catch (IOException e) {throw new ClassNotFoundException(name, e);}// Setup an in-memory writer for the byte-`ClassWriter writer = new ClassWriter(false);// Determine if we need to modify the classClassVisitor visitor = writer;if (name.equals(ThreadSafe.class.getName())) {visitor = new ClassModifier(writer);}// And load itreader.accept(visitor, false);byte[] byteCode = writer.toByteArray();return defineClass(name, byteCode, 0, byteCode.length);}}/*** Removes synchronization from a method. Currently only removes the `synchronized` access flag* from the method declaration.*/private static final class ClassModifier extends ClassAdapter {public ClassModifier(ClassVisitor visitor) {super(visitor);}public CodeVisitor visitMethod(int access, String name, String desc, String[] exceptions,Attribute attributes) {int newAccess = access;if ((access & Constants.ACC_SYNCHRONIZED) != 0) {newAccess -= Constants.ACC_SYNCHRONIZED;}return super.visitMethod(newAccess, name, desc, exceptions, attributes);}}}

There are two tests defined. One for success and one for failure as I mentioned at the start.

The aptly named testSucceedsAsis method executes a number of threads through a single instance of an unmodified class and hopes for the best :-).

Whilst probably non-obvious at first, the method testFailsWithoutSynchronization ensures that the test executes against an unsynchronized version of the class. It does this by creating a custom class loader that removes method synchronization through byte-code manipulating.

Some points of note off the top of my head:

• Although it would be almost trivial to add support for removing all synchronization from the code, for purposes of this excercise that wasn’t necessary;
• It may be necessary to inject random sleeps into code to try and get it to break as quickly as possible.
• Class loading is notoriously problematic and may become too difficult for more complex classes with many dependencies; and;
• I’m almost positive some AOP weenies will have something to add ;-).

This was largely an academic excercise and even though I clearly made a lot of assumptions, overall I was pretty happy with the outcome. We have demonstrated that it IS possible to test the thread-safety of a class. Whether this approach can be extended usefully to real-world examples remains to be seen. I leave that as an excercise for the reader ;-)

I found an article detailing how to add a comment feed to a Movable Type blog. I made a few changes (as one does) and now you can subscribe to the comments on this blog as well as the main feed. So for anyone who’s interested, here’s the template:

<?xml version="1.0" encoding="<$MTPublishCharset$>"?>
<rss version="2.0" xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:sy="http://purl.org/rss/1.0/modules/syndication/" xmlns:admin="http://webns.net/mvcb/" xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
  <channel>
    <title><$MTBlogName remove_html="1" encode_xml="1"$> - Comments</title>
    <link><$MTBlogURL$></link>
    <description>Latest comments on <$MTBlogDescription remove_html="1" encode_xml="1"$></description>
    <dc:language>en-us</dc:language>
    <dc:lastBuildDate><$MTDate format="%Y-%m-%dT%H:%M:%S"$><$MTBlogTimezone$></dc:lastBuildDate>
    <admin:generatorAgent rdf:resource="http://www.movabletype.org/?v=<$MTVersion$>" />
    <sy:updatePeriod>hourly</sy:updatePeriod>
    <sy:updateFrequency>1</sy:updateFrequency>
    <sy:updateBase>2000-01-01T12:00+00:00</sy:updateBase>
    <MTComments lastn="20">
      <item>
        <title>Comment on "<MTCommentEntry><$MTEntryTitle remove_html="1" encode_xml="1"$></MTCommentEntry>"</title>
        <link><MTCommentEntry><$MTEntryLink$></MTCommentEntry>#comments</link>
        <description><$MTCommentBody remove_html="1" encode_xml="1"$><p>- <$MTCommentAuthor remove_html="1" encode_xml="1"$></p></description>
        <guid isPermaLink="false">comment<$MTCommentID pad="1"$>@<$MTBlogURL$></guid>
        <dc:pubDate><$MTCommentDate format="%Y-%m-%dT%H:%M:%S"$> <$MTBlogTimezone no_colon="1"$></dc:pubDate>
      </item>
    </MTComments>
  </channel>
</rss>

I was giving a talk today on design and testing, refactoring, tools etc. and a question regarding the testing private methods came up.

We were discussing reducing cyclomatic complexity by encapsulating conditional statements in private methods. I showed a block of fictional code that included a test that the customer was at least 18 years old and contained a number of conditionals including the 2 or 3 checks required to test the age.

So we re-factored the code to extract the age check and demonstrated that the code was now much more readable and understanable. The newly created isAtLeast18YearsOld(Date dob) method made it clear what the calling method was trying to achieve.

The next question was “should we write a test for this?” and if so, “how are we going to do that if it’s private?”

Now, I almost never test private methods. I say almost, just in case i’ve either done it once before and forgotten or I need to change my mind at some point in the future. But as far as I’m aware, I never test private methods.

Private methods are private for a reason. They’re implementation detail. Yet another reason I dislike Java Beans so much - they simply expose the innards of classes such that the private instance fields might as well have been marked as public. “Guns don’t kill people, people kill people.” True enough but give a developer a getter and it’s death to good design.

Naturally, my first reaction is that I do believe there is enough logic in there to warrant a test but that it’s private and I don’t test private methods. Instead, maybe the method deserves to be public and therefore testable and if so where does it belong?

“how about we put it into the Person class?” someone asks. What a sensational idea I replied! No more passing around a Date.

Sometimes it’s more natural to place the the logic in say a strategy, making it pluggable. In this case you may choose to make the method more general such as meetsAgeRequirements(). Then, once it’s pluggable you could move the real implementation into a rules engine. Whatever you do, resist the temptation to put it into a static helper! IMHO statics are the last resort of the scoundrel programmer ;-).

In one simple example we’d managed to:

• greatly simplify our code;
• extract and make obvious some business logic;
• put that logic back into the class where it’s closest to the data on which it operates;
• Justify making the method publicly accessible and therefore testable; and;
• remove (from the class we’re implementing) a dependency on data (ie date of birth) contained within another class.

We’ve given our classes behaviour!

Private methods exist primarily to reduce the complexity of other methods and/or to remove code duplication. Either way, they are incidental to the implementation detail.

If you feel you need to test a private method (maybe because it’s complex or contains some kind of business logic), rather than subverting Javas access protection mechanisms or perverting your code, have a think about what the method really does and where it belongs. Chances are, you’ve missed an important abstraction or concept.

I remember having a heated discussion many years ago over the use of hungarian notation. Their argument went something like:

…If I don’t call it pLAmount, how do I know it’s a long?

To which I replied:

You don’t have to know if you create a Money class!

Whenever I see variables with type information embedded in their name or logically grouped by some prefix or suffix I immediately think there must be a missing class. An abstraction or simple concept that we haven’t expressed in code yet.

As an example, we’re in the process of building a booking system and we obviously needed a way to represent date ranges for various things such as, you guessed it, Bookings.

The first thing that emerged was a Booking containing a fromDate and toDate. This had two aspects (no, not AOP-speak) that annoyed me. One was the fact that the variable names all had suffixes of Date. The other, anytime we need to pass these values around, we need two, count ’em, 2 parameters for every date range!

Oh one more thing. I loathe and detest the java.util.Date classes. Many people have commented on the problems with them so I’ll say no more.

Instead we created a TimePeriod holding the start and end of the period represented as milliseconds GMT.

First the tests:

public class TimePeriodTest extends TestCase {
  public TimePeriodTest(String name) {
    super(name);
  }

  public void testSameRangeOverlaps() {
    assertTrue(overlaps(1, 2, 1, 2));
  }

  public void testRangeBeforeDoesntOverlap() {
    assertFalse(overlaps(1, 2, 10, 11));
  }

  public void testRangeAfterDoesntOverlap() {
    assertFalse(overlaps(10, 11, 1,2));
  }

  public void testRangeInsideOverlaps() {
    assertTrue(overlaps(5, 6, 1, 10));
  }

  public void testRangeOverlapsAll() {
    assertTrue(overlaps(1, 10, 5, 6));
  }

  public void testRangeOverlapsStartOnly() {
    assertTrue(overlaps(1, 6, 4, 10));
  }

  public void testRangeOverlapsEndOnly() {
    assertTrue(overlaps(4, 10, 1, 6));
  }

  private boolean overlaps(long f1, long t1, long f2, long t2) {
    return new TimePeriod(f1, t1).overlaps(new TimePeriod(f2, t2));
  }
}

And here’s the class itself:

public final class TimePeriod implements Serializable {
  public static final long FOREVER = Long.MAX_VALUE;

  private final long _from;
  private final long _to;

  public TimePeriod(long from, long to) {
    Assert.isTrue(from <= to, "from can't be > to");
    _from = from;
    _to = to;
  }

  public boolean overlaps(TimePeriod other) {
    Assert.isTrue(other != null, "other can't be null");
    return _to >= other._from && _from <= other._to;
  }

  public int hashCode() {
    return (int) (_from ^ _to);
  }

  public boolean equals(Object object) {
    if (object == this) {
      return true;
    } else if (object == null || !getClass().equals(object.getClass())) {
      return false;
    }

    TimePeriod other = (TimePeriod) object;
    return _from == other._from && _to == other._to;
  }

  public String toString() {
    return getClass().getName() + '[' + from + '=' + _from + ", to=" + _to + ']';
  }
}

You’ll note, there is no getFrom() nor getTo() method. Not because we won’t eventually need them but because so far we don’t need them for the user stories we have finished. More importantly, not having getters and setters forces us to think about classes in terms of behaviour not data. A topic I’ve ranted on previously.

So, we implement the class and now a Booking has a TimePeriod during which it is active.

Searching for available resources becomes pretty easy. We simply search for resources with no booking that overlaps a nominated period.

Instead of placing a getPeriod() on Booking, we choose instead to add a overlaps(TimePeriod) method. Again, it’s not because we think we won’t need to eventually get/set the period but instead we eel it’s more meaningful to add this kind of behaviour.

Let me re-iterate: Getters and setters are EVIL!

The next thing we needed to do was retrieve bookings. Aside from using a referenceId, the users want to search by name and booking date. The trouble is, sometimes they (the customers) don’t remember the exact date:

Um…it was sometime in january when I made the booking

says Mr. Smith. So how do we do this?

At first we though we might need some specialised class that magically knows to ignore days when comparing itself to a TimePeriod. It then struck us that we already had everything we needed.

Given our example, the month of January can be represented by a TimePeriod where the from is assigned 2004/01/01 00:00:00.000 and the to is assigned 2004/01/31 23:59:59.999. This necessarily overlaps any TimePeriod that falls within or around January 2004.

So now we can search for any booking that overlaps the specified period.

“Nothing staggering here” you might say. “I’ve seen all this before” mutters the crowd. Well true. But it serves as the basis to illustrate something I feel is very important in software design.

“Which would be?”

Continually simplify your code by introducing abstractions whenever and wherever possible. Abstractions, often at what appear to be ludicrously fine levels of granularity, almost always lead to better quality code. It’s much easier to remove an abstraction than to introduce one later on!

It’s always nice to find something about which I know bugger all. So today, instead of my usual rhetoric, magic answers and dubious words of advice, I roll on my back and display my soft under belly.

You see, I’m trying to make a class thread-safe. Almost all of my classes are inherintly thread-safe by virtue of the fact they are immutable. This class however updates internal data structures such as Maps and Sets.

Although Maps and Sets can be made internally thread-safe (by calling Collections.synchronizedX(anX)), that doesn’t really help when performing multiple calls (add(), remove(), etc.) on multiple structures (including instance variables) and treating them as one atomic-ish operation.

Now, I don’t want to just put synchronization blocks everywhere and hope for the best. I want to validate that the class is actually thread-safe. But I soon realised I have very little idea how to unit test for concurrency and thread-safety.

A quick search of the ‘Net turns up a few interesting links:

• GroboUtils
• Identify Problems with Testing Multithreaded Code
• JUnit best practices

But these only address the problems associated with running multiple threads within a test. Unfortunately, simply running multiple threads doesn’t actually prove that we have achieved thread-safety.

Because of the non-deterministic nature of thread scheduling, it is very difficult to ensure that we have had multiple threads accessing a single object concurrently, save adding hooks into the object itself to make it sleep or give up control at strategic points in the code.

Immediately, my brain kicks into golden hammer mode and decides that this problem looks like a BCEL or ASM nail. But, being the naturally lazy git that I am, writing code or having to deal with AspectJ doesn’t really appeal to me right now. Nor can I come up with any heuristics to determine where I would inject code anyway.

As you all know by now, I scoff at the idea that something is too hard to test. That said, I’m still left pondering how the hell I’m going to validate that my code is thread-safe? More to the point, how am I going to (re)design my class so that it’s easy to test?

I’ve finally finished my foray into C# and I suppose it would be obvious to all that I was rather less than impressed by some “decisions” that were made by the C#/.Net development team(s). Most noteably, the lack of checked exceptions.

So it was with much joy that I stubled upon this article, on the Artima web site. An interview with Anders Hejlsberg, the lead C# architect and a distinguished engineer at Microsoft, on “The Trouble with Checked Exceptions”.

Fantastic I thought! At last I’ll get some sensible, logical, coherent and rational explanation for some of the stuff that feels so uncomfortable to a Java weenie such as myself.

If only.

Thinking that maybe it was just me, I forwarded the link on to a good friend of mine James Ross who knows quite a lot about the Microsoft world. But alas, he drew similar conclusions. (Portions of our conversion have been included here)

No, it is sad to say but Mr. Hejlsberg shows his true colours and in the process makes me even less impressed with .Net.

I’m usually not fond of taking an argument and disecting it line-by-line. It’s often too easy to take stuff out of context and often leaves one open for a counter argument in a similar vein ultimately leading to a flame war. But on the basis that Mr. Hejlsberg wouldn’t know me from a bar of soap let alone read my blog, why not.

So without further ado:

…I completely agree that checked exceptions are a wonderful feature. It’s just that particular implementations can be problematic. …I think you just take one set of problems and trade them for another set of problems.

So he likes them but thinks that it’s the implementation of them that’s wrong. Um…I must be really stupid but HOW ELSE DO YOU IMPLEMENT CHECKED EXCEPTIONS? I can’t think of a simpler way than in Java. In fact I can’t think of any other way really. Please enlighten me!

Skipping foward a bit, he reckons

The concern I have about checked exceptions is the handcuffs they put on programmers … It is sort of these dictatorial API designers telling you how to do your exception handling.

Well how about being dictatorial about what methods you can override? In C# a method cannot be overriden unless you declare it as virtual.

He then goes on to say:

Let’s start with versioning, because the issues are pretty easy to see there. Let’s say I create a method foo that declares it throws exceptions A, B, and C. In version two of foo, I want to add a bunch of features, and now foo might throw exception D. It is a breaking change for me to add D to the throws clause of that method, because existing caller of that method will almost certainly not handle that exception.

Dude, try making an API that will be able to respond, like wrapping those FOUR DIFFERENT EXCEPTIONS into one abstract package-level exception, you clown! This argument doesn’t stand up to scrutiny because the same thing can be said about method parameters and return types, so why not get rid of them too!?

In fact he says:

C# is basically silent on the checked exceptions issue. Once a better solution is known - and trust me we continue to think about it - we can go back and actually put something in place…And so, when you take all of these issues, to me it just seems more thinking is needed before we put some kind of checked exceptions mechanism in place for C#.

Right! You’re somehow going to go back and change the way exceptions work!? Get real my dear architect. Whatever happened to your argument about versioning issues?

…in a lot of cases, people don’t care. They’re not going to handle any of these exceptions. There’s a bottom level exception handler around their message loop. That handler is just going to bring up a dialog that says what went wrong and continue. The programmers protect their code by writing try finally’s everywhere, so they’ll back out correctly if an exception occurs, but they’re not actually interested in handling the exceptions.

AHA! Now we begin to see the truth. He doesn’t actually like catching exceptions after all. I dont know what planet he is on but really.

I can just see my nuclear power station monitoring system catching a CoreMeltDownException in the “main message pump” and bringing up a dialog kindly informing the operator he might have a problem. Meanwhile, blissfully unaware, the rest of the system continues on removing the control rods from the core.

And what’s with the finally's everywhere to “protect their code”? Holy cow. Don’t let this man near any system I’m likely to work on. I don’t know about you but I rarely need to write finally statements except maybe in some integration code. Who manages resources this way these days? I’m not saying I dont use them but my code surely isn’t as littered with them as he implies it would.

Then he loses all credibility by delving into some spurious arguments on “scalability”:

The scalability issue is somewhat related to the versionability issue…Each subsystem throws four to ten exceptions. Now, each time you walk up the ladder of aggregation, you have this exponential hierarchy below you of exceptions you have to deal with. You end up having to declare 40 exceptions that you might throw. And once you aggregate that with another subsystem you’ve got 80 exceptions in your throws clause. It just balloons out of control.

Get out of my face! I’ve never EVER seen this, even in the worst code bases I’ve had the misfortune to work on. For a start, the numbers he quotes are just ludicrous. But again I say, how about wrapping those FOUR DIFFERENT EXCEPTIONS into one abstract package-level exception!? How about designing a language and libraries that allow smart people to do smart things instead of one that makes it even easier for stupid people to do stupid things?

What’s probably even worse, is that now my SQLException propogates all the way from my database layer to my GUI layer. Some “clever” developer realises that he can catch the SQLException, check the ErrorCode for 9901 which he happens to know means a key violation (or whatever) and display some nice message to the user. Whatever happened to encapsulation and abstraction? I mean, I know abstractions can leak but this is Niagra Falls baby!

But that said, there’s certainly tremendous value in knowing what exceptions can get thrown…

Excellent. Well at least we agree on something. Damn shame that because C# has no way of declaring what exceptions are thrown, I have no way of knowing if there are any to be caught, let alone what they might be. Unless of course it says so in the documentation. Documentation gets out of date VERY QUICKLY.

When I started my porting effort, I was using the mono doco which is still incomplete. This meant in some cases I had no idea if nor what exceptions might need to be attended to. What’s even worse, none of the exceptions seem to extend any sane base class so if I decide I need to catch more than one but treat them all the same way (say some kind of IOException) I have a 3 catch clauses!

But I think we can certainly do a lot with analysis tools that detect suspicious code, including uncaught exceptions, and points out those potential holes to you.

Ahhh. Of course (slap myself on the head) why didn’t I think of that? After all what we really need is yet another tool! In fact let’s create an AOP Library for C#. Yeah. Then we can inject code into existing libraries to catch exceptions…the possibilities for this are endless ;-)

But seriously, exceptions form part of your API just like methods, interfaces, parameters, abstract data-types, etc. If you think about them in this way, they stop being scary and start being useful.

Some things to remember when using exceptions:

• Don’t use exceptions for flow control;
• Create sensible exception heirarchies;
• Never throw more than one class of exception from a method unless you are forced to. That is, if you throw more than one type of exception, make sure they all extend a common base class. That way clients can catch them and/or re-thrown then easily;
• Avoid throwing someone elses exceptions. Eg. Don’t throw SQLExceptions from your middle-tier. Wrap them. There is usually no reason for the GUI to know there was an SQLException.

Well you asked for it and here it is. Results from running the native, C#, flavour of Simian versus the Java flavour.

As I mentioned earlier, I had originally run the comparison on my linux machine using mono. As many people had pointed out, this was far from a “fair” comparison. Some people even suggesting that purely porting the code would result in poor performance. To this I reply fooey.

The test (and I use the term loosely) was performed on a DELL 2.0 GHz Inspiron 4150 with 512MB RAM running Microsoft Windows XP Pro against the JDK 1.4.1_01 source.

And the winner is…I’ll let you be the judge:

The java version using the Sun JDK 1.4.2_03 ran in 64MB:

> java -jar simian.jar -recurse=*.java > java.txt
Similarity Analyser 2.1.0 - http://harukizaemon.com/simian
Copyright (c) 2003-11 Simon Harris.  All rights reserved.
Simian is not free unless used solely for non-commercial or evaluation purposes.
Loading (recursively) *.java from C:\jdk1.4.1_01\src
{ignoreCurlyBraces=true, ignoreModifiers=true, ignoreStringCase=true, threshold=9}
...
Found 40880 duplicate lines in 2339 blocks in 872 files
Processed a total of 369957 significant (1187603 raw source) lines in 3889 files
Processing time: 18.337sec

The C# version ran natively in 61MB:

> simian.exe -recurse=*.java > csharp.txt
Similarity Analyser 2.1.0 - http://harukizaemon.com/simian
Copyright (c) 2003-11 Simon Harris.  All rights reserved.
Simian is not free unless used solely for non-commercial or evaluation purposes.
Loading (recursively) *.java from C:\jdk1.4.1_01\src
{ignoreCurlyBraces=True, ignoreModifiers=True, ignoreStringCase=True, threshold=9}
...
Found 40880 duplicate lines in 2339 blocks in 872 files
Processed a total of 369957 significant (1187603 raw source) lines in 3889 files
Processing time: 12.628sec

Running with -server gains us about an 8% improvement in performance for the Java version but certainly still nothing like the nearly 30% needed to catch up to the native .Net

Surprisingly, running under BEA JRockit 1.4.2_03 used 235MB and tookaround 35 seconds using all default settings. The disk seemed to be thrashing but we made no attempt to tune the performance using JRockit options.

The C# version running under mono on the same hardware ran in 250MB and took around 78 seconds. Unfortunatele\y we couldn’t get any of the optimize features to work on the windows version of mono. Besides, we figure this comparison is rather moot. Rather it is better to compare Java versus Mono on linux.

So here are the results on a DELL 1.8GHz Inspiron 8200 with 1GB RAM running Gentoo Linux (2.4 kernel) against the JDK 1.4.2_03 source.

The java version using the Sun JDK 1.4.2_03 ran in around 60MB and took 25 seconds.

The C# version under mono (with -O=all) ran in around 90MB and took 34 seconds.

Amusingly, nay astoundingly, the .Net version runs natively faster under windows+VMWare+linux than the mono on straight windows or straight linux!! Go figure?

Interesting to say the least. I wait with baited breath for the ensuing storm of abuse from the Java community this entry generates Hehehe. Though it’ll make a change from receiving a serve from the .Net community.

Now the task is to see if we can pin-point what accounts for the difference. Unfortunately I fear, that because it’s a direct port (ie line by line), any improvements I make to the Java version will likely carry forward into the .Net version.

Performance aside .Net is still not my bag baby. It still feels a little clumsy. But then I’ve years of practise getting my Java up to scratch.