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:
- Am I doing it right?
- Is there an easier way to do it?
- 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.
17 comments:
OK, closures, yet how's a local variable not acceptable in this place?
Following code seems better to me because MutableInteger was too much of an incomprehensible hack against your numCars being final :
public static int getNumCarsInXml(XmlType xmlFile) {
CarUtils.CarFunction function = new CarUtils.CarFunction() {
private int numCars;
public void execute(CarXmlType xmlCar) {
numCars++;
}
}
CarUtils.forAllCarsInXmlFile(xmlFile, function);
return function.numCars;
}
Compare to public static int getNumCarsInXml(XmlType xmlFile) {
List cars = CarUtils.getAllCarsInXmlFile(xmlFile);
return cars.size();
}
Because java does not have closures yet, trying to force it can do a lot of damage to your code style. If there are more cars than can be reasonably returned, return an Iterator instead of a List.
A couple of style points:
1 - If you have create an object like your MutableInteger, rethink your strategy.
2 - Less code is better than more code. Unfortunately, nested inner classes add a bunch of extra code that makes it hard to grok.
3 - Violates the "do the simplest possible thing" rule. Doing something because it is the shiny new thing does not make it a good idea. There will always be a new shiny to play with. Keep is simple instead.
Oh, I almost forgot, be sure and check out the Jakarta Commons Collections for a bunch of useful functors in Java. I use them all the time.
You could instead have it produce a list of 1s for each car found, and then run a sum function on that list. It would be similar to the map-reduce word count example.
The simplest, albeit inelegant, way around the final restriction is to declare a final one-element array:
public static int getNumCarsInXml(XmlType xmlFile) {
final int[] numCars = new int[] { 0 };
CarUtils.forAllCarsInXmlFile(xmlFile, new CarUtils.CarFunction() {
public void execute(CarXmlType xmlCar) {
numCars[0]++;
}
});
return numCars[0];
}
@mathew o. smith:
I agree your version is definitely better if all I want to do is get the number of cars but my version of CarUtils.forAllCarsInXmlFile() allows me to do anything on each car in the xml so it's more general and a way of abstracting away the accumulation of car elements in the XML (which may involve many nested for loops depending on how often Car elements can appear in the XML structure).
I will have a good look at Jakarta Commons Collections, thanks.
@anonymous (2):
Hmmm. That looks like a good way of avoiding the MutableInteger. I will try that, thanks!
@mathias ricken:
Yes, someone over at reddit already mentioned this - I was not aware of it - it's definitely the most compact solution. Not sure I like it though. Almost feels like more of a hack that MutableInteger!
@everyone:
Thanks for taking your time to comment!
CarUtils.cars(xmlFile).foldLeft(0, new Function2<Integer, CarXmlType>()
{
. . public Integer invoke(Integer acc, CarXMLType xmlCar)
. . {
. . . . return acc + 1;
. . }
})
Oh for crying out loud! Use Xpath with dom4j!
Integer countCars= (Integer)document.selectObject("count(//cars)");
Done!
>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.
"Iterator".
To other people: scrap commons collections (and for heavens, anything "commons") - use google collections.
Oh, by the way, there is no need to define such "MutableInteger" classes. Here is a mutable integer that can be used instead:
final int[] value = new int[1];
@The Narrator:
Given the choice I would've used XPath and do in my own code but in this particular part of the system the decision was made to use JAXB despite my concerns. From experience, XPath is definitely the way to go - surprisingly (as far as XML technologies go) it's a great language!
My experience with JAXB has been painful to say the least but that's a story for another day!
@Ricky Clarkson:
Wow! foldLeft - a bit to "Haskell" for me :) Seriously though, if I get stick for my code example then I imagine what the response would be if I start using foldLeft in my code! I would get the sack for sure! Nice example though, thanks for sharing it.
"a bit to[sic] "Haskell" for me"
What is it about Haskell-influenced code that you dislike?
Ricky Clarkson said: "What is it about Haskell-influenced code that you dislike?"
Nothing, I was just kidding. I was trying to make the point that foldLeft is way to cool for it to be acceptable in my work place!
This is really the underlying issue of this post. I am learning new concepts by studying languages such as Lisp and Haskell and trying to introduce a small amount of the functional style into my code at work, but I am consistently met with resistance from colleagues who don't understand and don't WANT to understand the concepts. I need to take small steps and so things like foldLeft are out of the question I'm afraid.
For what it's worth, when I introduced it at work I called it reduce, and called the Function2 Reduction.
reduce(listOfInts, new Reduction<Integer, Integer>()
{
. . public Integer reduce(Integer soFar, Integer newValue)
. . {
. . . . return soFar + newValue;
. . }
});
It helped that the version without reduce/foldLeft would have involved serious code duplication.
If you can at least have a basis at work that code duplication is a bad thing, then you can probably introduce things like this. You might also point at Google's Map/Reduce framework (and hence Hadoop).
@Rickey: I keep meaning to take a look at Hadoop. Thanks for the reduce example - as you say, if I can make a good case for it I should get away with it so I'll keep a look out for cases where I could use this. Thanks!
I think this code is fine except:
new CarUtils.CarFunction();
I would not put a Class into CarUtils,
which apparently is a Class with static Helper Methods.
Post a Comment