A Shell in the Perl



  • We just inherited a web and feed-based app, written in Perl, that has
    been cobbled together over the past 5 years or so by a "senior"
    developer who has been at the company for over 20 years.  We've
    been handed down some pretty bad apps over the past few years, but this
    one takes the cake.



    Here is one line of code that pretty much summs up the entire app:

    <font face="Courier New" size="2">if( !open(F2, "|cat >" . $tempactions) ) {</font>

    and it only gets better...



    The app runs about 20 data feeds to 3 other systems.  Each feed
    runs twice daily and has 3 phases, each of which has its own separate
    log file that gets emailed to us, chock full of debug and informational
    messages, with random error messages interspersed.  If something
    goes wrong, it's almost impossible to tell the errors from all the
    chaff.



    Each feed script is an exact copy of every other feed script in the
    system with some minor tweaks, in every function in the script
    Now remember, I did say "function".  Functions actually exist in
    all the scripts, and, there are many home-grown libraries that are
    included in each script too.  However, every function in every
    feed script is a duplicate of that same function in the other feed
    scripts.  I think the main concept of "function" got lost here
    somehow...  Let's just say that %40 of our daily function is taken
    up supporting all the errors that come out of this app.



    In addition, every possible action you could perform via a builtin Perl
    function that has existed since the dawn of time, is implemented via a
    call out to the system shell, for example:

    <font face="Courier New" size="2">system("rm $temp2\n");  # Remove temporary files</font>

    that's just one example, here's another (anyone ever hear of Net::FTP?):

    <font face="Courier New" size="2"> print "open '|ftp -n -v $ftp_machine > $temp_file'\n";

     open (FTP, "|ftp -n -v $ftp_machine > $temp_file\n");

     print FTP "user $user_id $user_pw\n";


     print FTP "lcd $source_dir\n";

     for ($i = 0; $i < @filesource; $i++) {

       print "put $filesource[$i] $filesink[$i]\n";

       print FTP "put $filesource[$i] $filesink[$i]\n";

     }

     print FTP "quit\n";

     close (FTP);

     print "\nFTP completed.\n";


     # Here's the part I just love


     #

     #  Check the output file to make sure there are two instances of

     #  the string 'Transfer complete.';

     #

     my $grep_results;

     chomp($grep_results = grep -c 'Transfer complete.' $temp_file);

     if ($grep_results == 2) {

       return 1;  # It worked

     }

     else {

       system("cat $temp_file");

       return 0;  # It didn't work

     }

    </font>


    The majority of the scripts feed data to or from an Oracle database, sometimes using Oraperl and sometimes using DBI, depending on the whim of the day, sometimes both in the same script.  In addition, in these same scripts, there is one particular set of database queries that is run by creating a temporary file and calling Sqlplus via the system shell.  And so every single insert, delete, and update it does is echoed to the log file and hence included in an email to us.  And if you enjoyed the O'Hare problem in an earlier WTF, it's too bad you can't see the hand-coded quoting mechanism we have (too long to post here); some of our feeds come from user entered data from other companies' web sites.  There are half as many bound parameters as there are uses of this special "quoting" mechanism.

    Here's another little gem sprinkled throughout about half of the scripts (the other half don't use the "constant"):
    <font face="Courier New" size="1"><font size="2"> use constant TEST_MODE => 0;    # Turn on/off diagnostic messages

     $TEST_MODE = &TEST_MODE;        ## Test Mode (1 == ON, 0 == OFF)

     if ($TEST_MODE) {print "TEST_MODE is ON\n";}</font>

    </font>

    this appears at the top, and in the rest of the script, sometimes it's
    used as a constant, &TEST_MODE, and sometimes it's used as a
    variable, $TEST_MODE.



    Half of the "logging" in the scripts, is done via "print" statements
    directly to stdout, and the other half is done by calling a library
    function or two.  Both types of logging are sprinkled throughout
    every script, sometimes only one line apart.



    This is just the feed side of the app.  I don't even want to begin to mention the CGI part of the app.



    And to top it all off, where I work is supposed to be famous for its
    technological prowess and the number of bright stars it produces.



    -The Hermit






  • Apparently "refactor" was not in the original programmer's vocabulary.  Are you sure it was the same guy programming it each time?  I can't fathom how two different lines would do it two different ways.

    This looks like a great case for the TIAOASO* design school, although you may just want to try to clean it up first.

    (Throw it all away and start over)




  • @Benanov said:

    Are you sure it was the same guy programming it each time?  I can't fathom how two different lines would do it two different ways.

    Maybe he took "there's more than one way to do it" a bit too literally.



  • Aparently, he was schizophrenic...



  • I hate to be Devil's Advocate (ahh... who am I kidding) but it seems like the logical explanation is that he created the original (read: more flawed) code when he was first starting out with the company. Then as he progressed in PERL knowledge and was asked to modify the scripts, used his new knowledge to write "better" code. The fact that he's been at the company for 20 years gives a lot of room for him to be maintaining that code. Given a typical corporate environment I don't imagine he was give time to rewrite his older code.



  • Actually, the code was copied directly from another app around 5 years
    ago.  The feed scripts were created incrementally by doing the
    same thing, copying one feed script to another over a period of those 5
    years.  That was his idea of reusing code, instead of building a
    common function library for all the scripts to use, so the code didn't
    have to be copied (and fixed 20 times over when one bug was
    found).  What's just plain weird is that sometimes he did create
    functions for some things but sometimes he just copied and pasted.




  • @The Hermit said:

    Actually, the code was copied directly from another app around 5 years
    ago.  The feed scripts were created incrementally by doing the
    same thing, copying one feed script to another over a period of those 5
    years.  That was his idea of reusing code, instead of building a
    common function library for all the scripts to use, so the code didn't
    have to be copied (and fixed 20 times over when one bug was
    found).  What's just plain weird is that sometimes he did create
    functions for some things but sometimes he just copied and pasted.




    Maybe this guy has two brains?  =P

    I'm also having "fun" with an application I inherited in VB6 with tons of stored procedures (SQL Server)

    Weee legacy apps FTW!

    Mike Rod



  • It's programmers like this that give perl a bad name.  That, and the ones that like to be clever for cleverness' sake.
    I see at least one my... did he know enough to use strict and still write this?

    I'm sorry.



  • I'm sorry to say that I recognize this kind of programming.
    There are one or two guys where I work who tries to write everything in some sort of shell script.
    This kind of code is what happens when they're forced to write perl...



  • Yikes! Any given line looks like a shell scripters first week of perl, but to blissfully plow on for years, never getting better. The use of constant is nice. Someone must have ninja-taught him how to use it - well how to invoke it anyway.



    I've been (ab)using perl for a medium size web application the past few years. As such, I can go 6 months without writing a regular expression, and years without explicit filesystem I/O. It gave me great, sadistic, pleasure to check the following bit into CVS as part of a proof-of-concept test last week.



    `for a in *.static; do ../static_maker.pl \$a; done`;


  • The previous perl "programmer" at my current job had a copy of the same mailing list script for each of the mailing lists at the department, with the only difference between them being its name and a hard-coded list name encoded in the file.

    I replaced them all with one version that took an argument.  I also removed a lot of system() calls and backticks with native perl functions.  I suspect he was really only familiar with shell programming, but used perl for the regexes.



  • @The Hermit said:

    <FONT face="Courier New" size=2>system("rm $temp2\n");  # Remove temporary files
    </FONT><FONT face="Courier New" size=2>open (FTP, "|ftp -n -v $ftp_machine > $temp_file\n");</FONT>

    What's with the \n at the end of some of the shell commands?  Does it improve performance by 15 per cent or something?  Should I adopt the practice of appending newlines to shell commands, too, when someone happens to want me to write something in Perl for a change, and I just can't seem to remember what the Perl equivalent of <FONT face="Courier New" size=2>rm</FONT> might be?



  • @GeekMessage said:

    @The Hermit said:
    <font face="Courier New" size="2">system("rm $temp2\n");  # Remove temporary files
    </font><font face="Courier New" size="2">open (FTP, "|ftp -n -v $ftp_machine > $temp_file\n");</font>

    What's with the \n at the end of some of the shell commands?  Does it improve performance by 15 per cent or something?  Should I adopt the practice of appending newlines to shell commands, too, when someone happens to want me to write something in Perl for a change, and I just can't seem to remember what the Perl equivalent of <font face="Courier New" size="2">rm</font> might be?



    The newline does nothing ... most shells (all that I know of) parse on whitespace so the newline will never be seen by the ftp command. I think the scripter may be confusing the newline needed on a perl die statement (open(....) | die "error: $!\n") - the newline there prevents the script line and input line number from being printed.


  • @GeekMessage said:

    @The Hermit said:
    <font face="Courier New" size="2">system("rm $temp2\n");  # Remove temporary files
    </font><font face="Courier New" size="2">open (FTP, "|ftp -n -v $ftp_machine > $temp_file\n");</font>

    What's with the \n at the end of some of the shell commands?  Does it improve performance by 15 per cent or something?  Should I adopt the practice of appending newlines to shell commands, too, when someone happens to want me to write something in Perl for a change, and I just can't seem to remember what the Perl equivalent of <font face="Courier New" size="2">rm</font> might be?



    unlink is the perl (and c) equivalent of rm (for files).


  • I think you're being generous by calling this thing an "app" at all.  Tangled bunch of scripts is probably a more accurate description. 



  • @The Hermit said:

    that's just one example, here's another (anyone ever hear of Net::FTP?):

    <font face="Courier New" size="2">...

    </font>
    The majority of the scripts feed data to or from an Oracle database, sometimes using Oraperl and sometimes using DBI, depending on the whim of the day, sometimes both in the same script.


    To be fair, this sort of thing usually happens because the scripts are old, and the various modules had differing levels of existence, functionality, and bugginess at different times during the development of the system. It can often make more sense to write new code using a different one because it has some new features or fixes that make it work better, but there isn't enough time to rewrite everything to use that module. That's pretty much par for the course for any long-lived application, regardless of language.

    The rest is pretty awful though.


  • Either the CPAN module sucked, or the "developer" was one of those that had a severe mistrust of any third party library.

     "CPAN Modules?  Those are a bunch of bloated hunks of crap that are slow!  Why all that when all I need is to FTP!!"

     So many cowboy coders in perl land it really makes me ashamed to be a perl guy.

     


Log in to reply