jhourcle

Thank you very much for your comments. I will work on a new version of the script, following your suggestions, and I will post it some time next week.

I will address some of your comments below.

"I'm going to guess that this is a straight transfer of a C program to Perl --"

This is a very good guess. In fact, it is true! I have been programming in Perl for three months now and I recognize that I have a long way to go. This is one of the reasons I am asking for comments. So, I can improve my Perl coding skills in the least amount of time possible.

"First, you'd using a whole lot of for loops for tracking indexes to arrays:"

that is true. I am trying to overcome that habit

"In Perl, if you're just trying to iterate over a range, you can use the 'foreach' style loop, with the range operator:

for my $i ( 0 .. $number_of_clusters-1 ) { ... }
[download]

"Even if we were doing this in C, for the type of loops you're dealing with (starting at 0, order of operations doesn't matter), I'd still change the code, to reduce the number of comparisons against non-0 values:"

for (i = number_of_clusters; i--; ) { ... }
[download]

I like the two options. However, maybe the first one is easier to understand for someone who is new to Perl. For the second one, you must have clear that in Perl the evaluation of $i is done first allowing the loop to continue and then the variable is decreased. This might be hard to see for someone new to the language (I had to try it to see what it did)

"Another change I might make is in how you deal with undefined values -- if the value must be defined, and can't be 0, (eg, $number_of_clusters), you can use the '||=' operator:"

$number_of_clusters ||= 2;
[download]

thank you for the pointer. Trying the ||= operator made me realize that the $number_of_cluster cannot be negative either. So maybe I should do

my $number_of_clusters   = abs(shift @ARGV);
[download]

followed by the line you suggested. Is there another way around that?

"The only other thing is in how it's called -- if it were OO, you could inherit from it, and then replace the 'distance' function (or you could have it accept a coderef in for the distance function, if you didn't want to support inheritance), as some people prefer the manhatten distance when they're dealing with clusters:"

I have to think about this. I have to study OO in Perl, first.

Thanks again.