Wednesday, 22 October 2008

Oi, Java programmers... Is this code acceptable?

public static int getNumCarsInXml(XmlType xmlFile) {

    class MutableInteger { public int value; }

    final MutableInteger numCars = new MutableInteger();


    CarUtils.forAllCarsInXmlFile(xmlFile, new CarUtils.CarFunction() {

        public void execute(CarXmlType xmlCar) {

            numCars.value++;

        }

    });

    return numCars.value;

}


Note: I am using JAXB to transform XML elements into plain old java objects so just think of CarXmlType as a normal java bean.


This is an attempt to bring some of the functional style of programming I have adopted from learning Clojure to my day job.  I haven't really tested the approach much or given it extensive thought yet - just putting it out there to see what traditional object-oriented folk think of the style!

 

I like it because I have abstracted the details of accumulating all the "cars" away into forAllCarsInXmlFile() and it's all localised in the single function where it belongs.  The MutableInteger feels like a bodge though.  It's a rather crude way of getting around the way Java's inner classes work.  Questions which spring to mind are:

  1. Am I doing it right?
  2. Is there an easier way to do it? 
  3. Is this code as easy to read as alternative, more idiomatic alternatives?

Of course, what I really want is a language that fully supports Closures! Unfortunately, I can't use Clojure at work so I'll just have to wait for Java 7.








Thursday, 4 September 2008

Abstracting Away Patterns In Java

Today, while hacking away at my blub code in my day job, I noticed a very simple pattern in the code.  My first instinct was to create a utility function to abstract away the pattern, then I thought better of it after thinking for a moment about the inevitable effort and whether it would be worth my time - I have a tight deadline after all!  Then, I thought - sod it - lets see how painful this will be.  The end result?  It was quite painful!  This post explains why....


The pattern

Here's the code with the reoccurring pattern:


public void addReserveDog(String string, int ngrcId, int studBookId) {
    ReserveDogsXmlType xmlReserveDogs = xmlMeeting.getReserveDogs();
    if(xmlReserveDogs == null) {
        xmlReserveDogs = new ReserveDogsXmlType();
    }
    xmlMeeting.setReserveDogs(xmlReserveDogs);
}

All over the code I am doing this with different JAXB types.  In this case, I need to add something to the xmlReserveDogs element but it may be null.  So I need to get the element, check for null and if so create it, then set it again.

This same pattern happens everywhere but for different types.  Ideally, I'd like to abstract this pattern into a function such as this:

ReserveDogsXmlType xmlReserveDogs = getOrCreateXmlObject(xmlMeeting, ReserveDogsXmlType.class)

After messing around for a while I came up with this:

ReserveDogsXmlType xmlReserveDogs = (ReserveDogsXmlType) getOrCreateXmlObject(xmlMeeting, ReserveDogsXmlType.class, "getReserveDogs", "setReserveDogs");

I needed to pass in the names of the getter and setter methods to get it to work, but I can live with that.  The problem was the pain involved in getting this to work.  

Here is the implementation code:

public static Object getOrCreateXmlObject(
    AbstractXmlObject xmlParentObj, 
    Class<?> xmlObjectType, 
    String getterMethodName,
    String setterMethodName) {

    Object newXmlObj = ReflectionUtils.invokeMethod(xmlParentObj, getterMethodName);
    if(newXmlObj == null) {
        newXmlObj = ReflectionUtils.newInstance(xmlObjectType);
    }
    ReflectionUtils.invokeMethod(xmlParentObj, setterMethodName, xmlObjectType, newXmlObj);
    return newXmlObj;
}

Note that I use several utility methods here.  These are listed below:

public static Object invokeMethod(Object obj, String methodName) {
    Object result = null;
    try {
        result = obj.getClass().getMethod(methodName).invoke(obj);
    } catch (Exception e) {
        ////.... error handling .....
    }
}

public static Object invokeMethod(Object obj, String methodName, Class<?> paramType, Object arg) {
    Object result = null;
    try {
        result = obj.getClass().getMethod(methodName, paramType).invoke(obj, arg);
    } catch (Exception e) {
        ////.... error handling ....       
}

public static Object newInstance(Class<?> xmlObjectType) {
    Object newObj = null;
    try {
        newObj = xmlObjectType.newInstance();
    } catch(Exception e) {
        ////.... error handling ....
    }
    return newObj;
}


Summary

It is obvious from this experiment that the effort involved in abstracting away simple patterns is quite high in comparison to the benefits.  However, I still think it's worth it.  As the code base evolves more and more patterns will be abstracted and there will be less and less code duplication which should make code easier to maintain.  Also, the ReflectionUtils can be reused elsewhere in the code.  


Tuesday, 11 December 2007

Bottom-Up Development - Embracing Change

I have just read Raganwald's post, Something's Fishy, and it made me start thinking hard. One statement in particular caught my attention:
"Now maybe this [bad software] has absolutely nothing to do with programmers."
which spurred me to start writing. I was going to add my thoughts as a comment on Raganwald's blog but once I started writing I couldn't stop! Also, I have just started this blog so I thought, why not - I'll publish my brain-dump here.

In my experience, the fact that writing software is hard is the ultimate driver for all the bureaucracy trust upon developers in medium to large software houses.

Managers are scarred by past experiences maintaining bug-ridden, badly designed systems so they inevitably try to find ways minimize risk by enforcing rule upon rule. They embrace the fear of "getting it wrong" and introduce layers of process from pre-planning to lengthy design phases, to requirements re-re-re-analysis and so on. Often the rules put in place are ad-hoc, subjective and designed to prevent recurrence of specific nightmares from the past which will probably never reoccur again, at least not in exactly the same way.

I believe putting blame on bad management and bureaucracy is generally futile because, while not necessarily incorrect in specific circumstances, they are just symptoms of the real problem. The real problem in my opinion is that - even when using popular dynamic languages like Python - writing bad code is way too easy to do and maintaining it can become a nightmare as time goes on and feature creep has it's wicked way with an infrastructure never designed to support the features it is being asked to support.

That's why I am so excited about discovering the power of Lisp and in particular when Paul Graham talks about bottom-up programming. I believe the main players in the software industry focus far too much on top-down design of extendible and reusable systems. This is the basis for OO and it has never, ever sat well with me. How can we, in the real world, really expect anyone to come up with a system that is designed for change when we have no idea what the change will be? We can guesstimate and plan ahead and sometimes we may get it right, but there always comes a time in a system's lifetime when the customer requests a feature that "the system wasn't designed for".

I am very new to Lisp and more generally, functional programming concepts, but when I read about bottom-up programming in On Lisp, it all makes so much sense! Maybe we shouldn't be designing for change? Maybe we should accept that code will change in ways we cannot predict, then embrace it, then begin to use tools (like Lisp) that help us to manage and maintain such code elegantly.

If tasked with writing a geographical mapping product that tracks ship movements, do we spend months writing an infrastructure to support generic objects and potential features that the customer hasn't asked for, but may well do in the future? Or do we write a geographical mapping product that tracks ship movements? Your typical OO enthusiast we say, "Oh, you can't create a Ship class, you should be more generic than that! Study your Ship class for a while - I could pick several attributes and behavioural aspects of your class that aren't specific to a ship and come up with about five base classes. Wham Bam! An extendible and reusable system!".

But I just want to write some software that meets the requirements and more importantly... works. I want to keep it simple to reduce risk and minimise the chance of it not working properly. If I break-down my Ship class into multiple base classes I am not going to use (and repeat this approach throughout the system), then I am unnecessarily complicating the code and arguably wasting time and money.

Nevertheless, the OO guy has a point. If I limit my code to exactly what is required now, then it will be difficult to extend in the future. But, crucially, if the OO guy had his way and generalised the code using a top-down approach, unless he possessed psychic powers he would undoubtedly end up with a system that was just as difficult to extend than mine (except maybe in certain specific ways he had thought of), yet his system would be far more complicated and more difficult to maintain.

I believe the bottom-up approach is better because I end up with a simple system that works NOW! But I still have the problem that my code is very difficult to extend. What I need is a tool designed to aid, manage and maintain code written in a bottom-up style. Although I have no experience using Lisp in production environments, from what I have read so far I am pretty sure this is precisely where Lisp shines, primarily because of macros. Macros combined with other powerful features of Lisp provide the programmer with the flexibility to generalise code after the fact. It allows us to write very specific code that doesn't go any further than meet current requirements (you know - the ones that actually exist!). As the system evolves and new requirements are requested by the customer, Lisp allows programmers to elegantly mould their system into a new beast capable of meeting those requirements.

Now, if I were a Lisp expert I would dive into an example of it's power at this point. But I'm afraid I am just at the beginning of my journey towards Lisp Enlightenment. I have just purchased The Little Schemer, I am almost finished my first attempt at Pratical Common Lisp and I have read the first few chapters of On Lisp. So I have a long way to go, and time permitting I will try to record some of my learning experiences here.

Wednesday, 21 November 2007

Candidate For Best Comment Ever!

Today, I was given a bug report stating that one of the fields in the system should be 200 by default, yet for some reason it is showing up as 100. Due to my work load someone else investigated the bug before I had a chance to take a look and shortly after he came over to my desk with a big smile on his face.

When he pointed me at the offending code, I instantly realised why he was smiling. Here's my candidate for best comment ever:

static const long INITIAL_PING_SECONDS_REMAINING = 100; //TODO: Needs to be 200
For a few seconds I was stumped. This can't be my code! What was I thinking???? Then it hit me and I remembered what I'd done. I set the constant to 100 during development to make it quicker to debug so I put the TODO there to remind myself to put it back to 200 when finished. Nothing wrong with that of course, but I could have saved myself considerable embarrassment if a) the comment was a little more descriptive like "TODO: 100 for debugging, remember to change back to 200" maybe and b) if I actually checked my TODO's before committing!

Oh well, lessons learned I guess :-)

Thursday, 15 November 2007

The First Post

Argh! I can't take it anymore. I NEED TO GET THIS STUFF OUT OF MY HEAD! Here's a wild idea:

if I join the masses and start a blog I will be able to dump some of this stuff to make some room for other things (like a life, for a start) but not only that - if what I dump receives even a little exposure then maybe people will actually help me make sense of it! Wow, what a great idea!