Re^3: Squarepusher - A Tool To Convert Images To Audio For Oscilloscope X/Y Mode Displays

[Athanasius]

Can’t speak for jdporter, but to my way of thinking the loops (C-style instead of foreach-style) are the least of the problems. To my old eyes the major issues are:

1. $mode never changes, but it’s re-tested each time through the main loop.

2. The code is monolithic and should be refactored into subroutines (one for each mode).

3. use warnings; and use strict; are missing, and all the variables are global.

Hope that helps,

[jdporter]

In addition to the above:

Instead of long runs of single-line comments, use POD.

Instead of:

$mode=$ARGV[0];
$frames=$ARGV[1];
$lossy=$ARGV[2];
$skip=$ARGV[3];
@bitmap=`cat $ARGV[4]`;

How about:

my $mode = shift;
my $frames = shift;
my $lossy = shift;
my $skip = shift;
my @bitmap = <>;

Even better might be the use of a commandline option processor, e.g. Getopt::Long.

Instead of:

foreach $item (@bitmap)
{
  chomp($item);
  $total.=$item;
}
@image=split(//,"$total");

How about:

chomp @bitmap;
@image = split //, join '', @bitmap;

It's not clear why

            $col=$c;
            $row=$thisRow;

Such a construct usually implies protecting variables from being clobbered... but I don't see any clobbering going on...

# If you don't want to use this optimization, just
# comment out the whole if statement.

Why not provide a command line switch for it?