Beefy Boxes and Bandwidth Generously Provided by pair Networks
Think about Loose Coupling
 
PerlMonks  

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

by jdporter (Canon)
on Feb 02, 2013 at 01:17 UTC ( #1016630=note: print w/ replies, xml ) Need Help??


in reply to Squarepusher - A Tool To Convert Images To Audio For Oscilloscope X/Y Mode Displays

That's freakin' phenomenal. My first thought was: "And people think Perl's no longer useful!"

Which is why it pains me to have to tell you .... your Perl is ... um ... not great.


Comment on Re: Squarepusher - A Tool To Convert Images To Audio For Oscilloscope X/Y Mode Displays
Re^2: Squarepusher - A Tool To Convert Images To Audio For Oscilloscope X/Y Mode Displays
by exixx (Acolyte) on Feb 04, 2013 at 14:22 UTC
    To my old eyes it looks like FORTRAN. I assume you're talking about the loops. What could/should be done to make it better? Actually curious, I'm not good at this yet.

      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,

      Athanasius <°(((><contra mundum Iustus alius egestas vitae, eros Piratica,

        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?

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://1016630]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others romping around the Monastery: (5)
As of 2014-12-20 01:37 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    Is guessing a good strategy for surviving in the IT business?





    Results (95 votes), past polls