Unlike many of our esteemed monks, who obviously either have Computer Science degrees or are currently in Computer Science program, my formal programming education is essentially nonexistent. Although I've worked professionally as a programmer, as well as hiring, managing, and mentoring other programmers, I've never had an opportunity to have my code reviewed by other programmers who were qualified to give constructive criticism.

I've been a little hesitant to subject my code (and my ego) to public scrutiny, but it is a necessary step if I am to become a better programmer. I'm also hoping that it will serve the public good. :-)

(If there is nothing that others can learn from my code, then there will almost certainly be something to learn from the responses!)

This is a fairly typical program for me, though I usually write in a more modular style:

#!/usr/bin/perl -w ##################################################################### # exh_import.plx # By Brad Smithart (AKA Impossible Robot) # # $Revision: 1.1 $ # # Description: # Imports Exhibitor information from tab-delimited text # file into exh_search.cgi database tables. # Import file contains header row and data rows consisting of # a 'company' column followed by various optional columns # and any number of 'booth' columns. # ##################################################################### # Load Modules use strict; use DBI; use Getopt::Std; my (%opts, @insert_fields); getopts('cwfh', \%opts); # Get arguments unless (@ARGV > 1) # Check for correct number of arguments { my $program; ($program = $0) =~ s!.*[/\\]!!; # Could use File::Basename print <<"EOP"; Usage: $program [options] [input file] [database name] Imports exhibitor data into exhibit search database from tab-delimited text file. Options: -c Force case conversion (to title-case) -w Import URL data ('showurl' and 'web' fields) -f Import data using column headers for fields -h Expanded help (-w and -f options cannot be used together.) EOP print <<"EOP" if $opts{'h'}; Sample Import File Structures: ============================== Without '-w': +-------+-----+-----+-----+ |company|booth|booth|booth| +-------+-----+-----+-----+ |ABC Co.|1001 |2001 | | +-------+-----+-----+-----+ |DEF Co.|1002 | | | +-------+-----+-----+-----+ With '-w': +-------+-------+-------------+-----+-----+-----+ |company|showurl|web |booth|booth|booth| +-------+-------+-------------+-----+-----+-----+ |ABC Co.|1 |www.abcco.com|1001 |2001 | | +-------+-------+-------------+-----+-----+-----+ |DEF Co.|1 |www.defco.com|1002 | | | +-------+-------+-------------+-----+-----+-----+ With '-f': Uses column header names for field names; 'booth' fields must be in final columns. EOP exit; } die "\n**Error: '-w' and '-f' options cannot be used together.\n" if ($opts{'w'} && $opts{'f'}); my ($input_file, $database) = @ARGV; # Assign arguments to variables # Define Variables my $company_count = 0; # Database Variables my $dsn = "DBI:mysql:$database:localhost"; my $username = 'username'; my $enc_passwd = q{(<&%S<W=O>F0`}; # Not encrypted, just obscured my $passwd = unpack("u", $enc_passwd); my $exhibit_table = "exhibitor"; my $booth_table = "booth"; # Connect to database my $dbh = DBI->connect($dsn, $username, $passwd, { RaiseError => 1 }); # Print program identifier my $opt_text = $opts{'c'} ? '(with case conversion)' : ''; $opt_text .= $opts{'w'} ? '(with URL import)' : ''; $opt_text .= $opts{'f'} ? '(using field names from header)' : ''; print "Exhibitor Import $opt_text\n"; # Open file open(INPUT, "$input_file") or die "Couldn't open import file: $input_file\n"; my $header = <INPUT>; if ($opts{'f'}) { # Validate header $header =~ /^company/ or die "Invalid data - first column must be 'company'\n"; $header =~ /\tbooth/ or die "Invalid data - must contain 'booth' column\n"; # Remove booth fields, then get insert fields my ($fields) = split(/\tbooth/, $header); @insert_fields = split(/\t/, $fields); # I should probably also validate insert fields against database } else { @insert_fields = qw( company ); push(@insert_fields, qw( showurl web )) if $opts{'w'}; } # Setup queries my $sth_LastID = $dbh->prepare('SELECT LAST_INSERT_ID()'); my $insert_fields = join(',', @insert_fields); my $placeholders = join(',', map {'?'} @insert_fields); my $query_InsExh = <<"EOQ"; INSERT INTO $exhibit_table ( $insert_fields ) VALUES ( $placeholders ) EOQ my $sth_InsExh = $dbh->prepare($query_InsExh); my $query_InsBth = <<"EOQ"; INSERT INTO $booth_table ( booth_number, exhibitor_id ) VALUES ( ?, ? ) EOQ my $sth_InsBth = $dbh->prepare($query_InsBth); while (<INPUT>) { my (%data, @booths, @values); s/\"//g; # Remove all quotes (@data{@insert_fields}, @booths) = split(/\t/, $_); if (exists $data{'web'} && $data{'web'}) { $data{'web'} = 'http://'.$data{'web'} unless $data{'web'} =~ /^http\:/; } # Convert each word to ucfirst case $data{'company'} =~ s/(\w+)/\u\L$1/g if $opts{'c'}; $sth_InsExh->execute( @data{@insert_fields} ) && $company_count++; $sth_LastID->execute; my ($exhibitor_id) = $sth_LastID->fetchrow_array; foreach my $booth (@booths) { if ($booth =~ /\w+/) { $sth_InsBth->execute($booth, $exhibitor_id); } } } # Disconnect from database $sth_LastID->finish; $dbh->disconnect; print "----------\n$company_count companies inserted\n"; exit;

Please feel free (as I know you will) to criticize style, efficiency, etc. I would also be curious to hear how this might be tranformed into an OOP style (which I think would be a good exercise for me). Thanks for any advice you can give me.

Impossible Robot


In reply to For Review: DB Import by impossiblerobot

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.