Guys, I'm preparing a large new patch that fixes style problems in the
code, for after the code freeze. This is my last pass at this for
Mahout.
Style is not a big deal, though it's probably not good that random
non-standard Java is committed to the project. The only hard 'fix' for
this long-standing phenomenon is requiring a review process, and that
is too much. I don't think this project adheres to standards so much,
and such is life.
However, simply turning on code inspections in a modern IDE like
IntelilJ is turning up plain bugs in the code. I want to call out a
few, because I want to fix them (after 0.7), but also because I want
to make the point that static analysis can find bugs. Because it can,
it should. I think open source projects can and should be the finest
output of the best and brightest. And at "mere" Google, stuff that
static analysis finds would never have gotten to even code review.
Hence I am somewhat dismayed to see so many problems being committed
without review into the code base.
Here's a taste...
TestMeanShift.testCanopyEuclideanMRJob(), line 365
Vector[] permutedRaw = new Vector[raw.length];
for (int i = 0; i < raw.length; i++)
permutedRaw = raw;
Oops. I think this accidentally accomplishes its goal of a copy, but
it intends a deep copy and that's not what it does.
LocalSSVDPCADenseTest.runSSVDSolver(), line 107:
xi.assign(Functions.mult(1 / m));
m is an integer, so 1/m == 0 for any m > 1. This is not the intent,
but the test is actually crafted to work only when xi is assigned to
0.
Let's raise the bar please.
