Going through the code one more time before starting release for 0.2 and I have noticed some possible issues -- or else I miss something.
FPGrowth:258 : is this for loop really supposed to declare one variable but update another? this could be correct but reads as confusing LuceneIterable, SequenceFileIterable. The Iterators here have the same problem - hasNext() can't have side effects. next() is what does the updating. In these implementations hasNext() is what advances the underlying data structure. This isn't going to work in general. (In fact, might be good to look for "TODO" in the code where I have over time flagged things that look like a possible bug to me, mostly stuff that is updated but never read or something like that. Some may well not be; just remove the TODO then.) Also let's not call System.exit()? it's abrupt to the point of not necessarily running finalizers, which might be a bad thing. The return value of main() rarely matters, and, throwing an exception is a more conventional way to indicate failure and generate a non-zero return value. Also I find it confusing to give a generic parameter a name longer than 1-2 characters. It looks like a class name. and from the past ... No * imports Don't throw RuntimeException or Exception CONSTANTS should be final No serialVersionUID, it breaks things Conventional modifier order is 'public static final' Conventional order of elements of class file: statics, members, constructor, methods, inner classes Still lots of javadoc problems -- bad refs, empty elements it's small, but good to follow a standard coding convention, and keep the code clean.
