I’ve got a long-running project that’s very nearly completed, in which I’m converting some old IDL code to Java for the modeling of Martian photometry as observed by HiRISE. I use JFreeChart in this project (and all of my projects that require charts, in fact).

I happened to be digging into the source code for the NormalDistributionFunction2D class and I found this piece of madness:

public double getValue(double x) { return Math.exp(-1.0 * (x - this.mean) * (x - this.mean) / (2 * this.std * this.std)) / Math.sqrt(2 * Math.PI * this.std * this.std); }

“std” is the function’s standard deviation, set at construction.

The madness is in the denominator of the function: the square root of…a constant times the standard deviation squared. Isn’t that just the standard deviation times the square root of a constant? And the square root of a constant is itself a constant.

So why isn’t the denominator just a constant times the standard deviation? Why have four multiplications in there followed by a call to the math library? This expression is evaluated every single time the NormalDistributionFunction2D’s getValue method is called!

Because the standard deviation could be negative! Wait, what? No, it can’t. Even if there was some sense to it, the only computation in this code involves squaring the standard deviation, effectively stripping it of its sign. The denominator really ought to be a pre-computed constant times the standard deviation.

Also, since the class doesn’t let standard deviation be changed after construction, *the entire denominator can be pre-computed at construction*, as can the denominator of the exponent used in the numerator. Neither component ever changes once the object has been constructed.

Sigh.

This is in version 1.0.10 of JFreeChart, by the way. Maybe it’s been fixed by now. I probably ought to be a good open source citizen and submit a fix myself if it’s not been fixed.