Hi Marc,
On Saturday 11 June 2011 05:25:18 sono-io@fannullone.us wrote:
O.K. I've finished my code to create a MySQL db and/or table, and I'd
appreciate hearing your comments. It works well, but it needs to be
cleaned up before going live. I've left some testing code in. Go ahead
and be brutally honest. I've learned so much from this list already.
Very well, I'll comment on your code.



use warnings;
use strict;
use CGI::Carp "fatalsToBrowser";

use feature 'say';

use DBI();
Better have a space between the "DBI" And the "()".
use CGI;
my $q = CGI->new();
I personally distaste calling the CGI instance "$q".
say $q->header(), $q->start_html( -title => "Create MySQL db and table" );

my $username = 'root';
my $password = 'root';
my $host = "";
my $db = "test_dbb";
I hope you are not connecting to the database as user root with password root.
# table name is set below

# Connect to the server to create the db, if it doesn't exist already.
my $dbh_db = DBI->connect("DBI:mysql:database=;$host",
$username, $password,
{'RaiseError' => 1});

my $qry_db = "CREATE DATABASE IF NOT EXISTS $db ";
$dbh_db->do($qry_db) or die ("Can't create db!\n");
No need for the temporary variable. Just do:

$dbh_db->do("CREATE DATABASE IF NOT EXISTS $db ");
if ($dbh_db) { say "Database '$db' was created successfully.<br /><br
/><br />" }
That's the wrong way to test for errors.
# Disconnect from the database.
You should limit the scope of $dbh_db because it's not used after that:

my $dbh_db = DBI->connect(..)


# Connect to the database to create the table and fields.
my $dbh_table = DBI->connect("DBI:mysql:$db;$host",
$username, $password,
{'RaiseError' => 1});

my ($primary_key, @fieldnames);
my $file_path = "../htdocs/carts/sql/data/orders.tsv";
use File::Basename;
Put all the use's at the top.
my $table = fileparse($file_path, qr/\.[^.]*/);
What are you trying to do here?
open (my $file_fh, '<', $file_path) || die ("Can't open $file_path for
reading"); my $count = 0;
1. You should break these lines.

2. Put «my $count = 0;» on a separate line.
my $qry_table = "CREATE TABLE IF NOT EXISTS $table ( ";
while (my $line = <$file_fh>) {
last if $line =~ m/#/; # can place comments below the ### line
1. Is this a comment anywhere or only at the beginning of the string?

2. Always use last/next/redo/etc. with labels:


3. You should split your code into paragraphs.
chomp $line;
next if (! $line); # skip any blank lines in the file...
The "!" will also match 0. Maybe do:

if ($line eq '')
$line =~ tr|a-zA-Z0-9,_\t \(\)||cd;
my @fields = split (/\t/ , $line);
push ( @fieldnames, $fields[0] );
if ($count == 1) {
$primary_key = $fields[0];
my $key = $fields[0];
my $keytype = $fields[1];
1. You're using $fields[0] more than one time here.

2. You should unpack an array using:

my ($key, $keytype) = @fields;
if ($keytype) {
$qry_table .= "`$key` $keytype, ";
else {
$qry_table .= "`$key` TEXT, ";
1. This way you'll have a trailing L.

2. You can use «$keytype ||= 'TEXT'» instead here and have less duplicate
$qry_table .= "PRIMARY KEY (`$primary_key`) ) ENGINE=MyISAM DEFAULT
CHARACTER SET latin1 COLLATE latin1_general_ci;";

$dbh_table->do($qry_table) or die ("Can't create table!\n");
if ($dbh_table) { say "Table '$table' was created successfully.\n" }

close $file_fh;

say $q->end_html();


Shlomi Fish

Shlomi Fish http://www.shlomifish.org/
Escape from GNU Autohell - http://www.shlomifish.org/open-

Every successful open-source project will eventually spawn a sub-project.

Please reply to list if it's a mailing list post - http://shlom.in/reply .

Search Discussions

Discussion Posts


Follow ups

Related Discussions

Discussion Navigation
viewthread | post
posts ‹ prev | 2 of 6 | next ›
Discussion Overview
groupbeginners @
postedJun 11, '11 at 2:25a
activeJun 11, '11 at 7:32p



site design / logo © 2021 Grokbase