Hi Marc,
On Saturday 11 June 2011 05:25:18 sono...@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.
> Thanks,
> Marc
>
> ---------
>
> #!/usr/local/bin/perl
>
> 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 = "127.0.0.1:8889";
> 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.
> $dbh_db->disconnect();
>
You should limit the scope of $dbh_db because it's not used after that:
{
my $dbh_db = DBI->connect(..)
.
.
.
$dbh_db->disconnect()
}
>
> # 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:
http://perl-begin.org/tutorials/bad-elements/#flow-stmts-without-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 '')
{
next;
}
> $line =~ tr|a-zA-Z0-9,_\t \(\)||cd;
> $count++;
> 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
code.
> }
> $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();
>
> $dbh_table->disconnect();
>
Regards,
Shlomi Fish
--
-----------------------------------------------------------------
Shlomi Fish http://www.shlomifish.org/
Escape from GNU Autohell - http://www.shlomifish.org/open-
source/anti/autohell/
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 .
--
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/
