tel2 has asked for the wisdom of the Perl Monks concerning the following question:

Hi Monks,

I've just written my first (significant) package, which is intended to centralise connections to a database, and the running of queries against it.

Here's script1.pl, which is the first of several scripts which will call the package:

#!/usr/bin/perl

use crsTools4;

connect2db;

# Test query using $sth
run_sql("SELECT * FROM person WHERE firstname = 'Joe'");
while ((@f) = $sth->fetchrow())
{ print "@f\n" }

# Test query using $sth2 (though I could just reuse $sth in this case)
run_sql("SELECT * FROM person WHERE firstname = ? AND surname = ?",2,'
+Joe','Bloggs');
# while ((@f) = $sth2->fetchrow())
while ((@f) = $sth->fetchrow())
{ print "@f\n" }

[download]

And here's the package (filename crsTools4.pm):

package crsTools4;

require Exporter;
our @ISA    = qw(Exporter);
our @EXPORT = qw(connect2db run_sql); # Symbols to be exported by defa
+ult
our @EXPORT_OK  = qw($dbh $sth);

sub connect2db
{
        require DBI;
        our $dbh = DBI->connect("DBI:mysql:database=crs;host=127.0.0.1
+", 'crs_user', 'mypw') or die('Could not connect!');
}

# Execute an SQL command (arg 1), with an optional specified handle #
# (arg 2), and optional bind variables (args 3...).
sub run_sql
{
        my ($sql, $n) = (shift @_, shift @_);
        print "sql=\"$sql\", sth$n, bind=\"" . join(',',@_) . "\"\n";
        # $$sth = $dbh->prepare($sql);
        # $$sth->execute(@_) or die "Could not execute: $DBI::errstr\n
+";
        # To make things simple while debugging, let's just hard-code 
+$sth
        $sth = $dbh->prepare($sql);
        $sth->execute or die "Could not execute: $DBI::errstr\n";
}

1;  # Required for all packages, to return a true value

[download]

And, here's the output, complete with error message:

sql="SELECT * FROM person WHERE firstname = 'Joe'", sth, bind=""
Can't call method "fetchrow" on an undefined value at ./test4.pl line 
+9.

[download]

As you can see, it fell over on the first query.  Any ideas what I'm doing wrong?  I assume it's to do with localisation of $sth or something (though I tried to make it global), but after a lot of trial and error, I'm ready to take advice from the experts...please.

Thanks.

Replies are listed 'Best First'.
Re: Errors from a simple Package
by Eliya (Vicar) on Mar 16, 2011 at 10:58 UTC

    Personally, I would have run_sql() return the $sth handle, rather than having people who'll read this code later check the docs/source of the module in order to figure out what's the deal with the global variable.

    Much easier to read, IMHO, and you can also have multiple $sths if you need to.

[reply]
[d/l]
[select]
Re: Errors from a simple Package
by wfsp (Abbot) on Mar 16, 2011 at 09:50 UTC
    I think you need 
    use crsTools4 qq{$sth}; # import an EXPORT_OK var

    [download]
    in the script. The package should probably be 
    our @EXPORT_OK  = qw($dbh $sth);
our ($dbh, $sth); # declare vars to be EXPORT_OKed

    [download]
    and remove the our in the connect2db sub.

    I don't know where $sth2 would come from.

[reply]
[d/l]
[select]
Re: Errors from a simple Package
by tel2 (Pilgrim) on Mar 17, 2011 at 00:39 UTC

    Thanks heaps wfsp & Eliya!  I've made use of both of your advice, and it's working (just had to change 'qq{...}' to 'qw{...}' and it was all go).

    In response to wfsp's query about $sth2, sorry - when preparing the test script for posting, I'd missed the line: 

      my $sth = "sth$n";

    [download]
    which should have been after the 'print' line in 'run_sql()'.  If '2' is passed as the 2nd argument, $sth becomes 'sth2', so $$sth references $sth2.  But I'm not using that method anymore, thanks to Eliya's suggestion.

    One further question about that 'print' line, if I want to have it print to my DEBUG file, which was opened by script1.pl, how can I share that filehandle to the 'run_sql()' sub?  I've tried adding 'DEBUG' with & without a '*' prefix, to the lists of globals, to no avail.  I'd rather not pass the filehandle as an argument to 'run_sub()' for every call.

    Thanks.

[reply]
[d/l]
      Just have your debug logger open up the file for appending, e.g.:
      
use POSIX qw(strftime);

sub log_msg {
  open(my $fh, ">>", 'log_file.txt') or die "Err: $!";
  printf $fh "[%s] %s\n", strftime("%Y-%m-%d %H:%M:%S", localtime), sh
+ift;
  close $fh;
}

...

log_msg("Log this message");

      [download]
[reply]
[d/l]

        Thanks runrig,

        Good suggestion - I'm trying that now.  I hadn't considered opening the file evertime I want to append it.  Probably not a big overhead since I'll probably only be doing it when I'm trying to debug something.

        But do you know if/how I can share a filehandle (e.g. "DEBUG") between a script and a package?

        Thanks.

[reply]
Re: Errors from a simple Package
by runrig (Abbot) on Mar 17, 2011 at 01:18 UTC
    It seems to me that all of your connections and queries could be "centralized" using connect_cached and prepare_cached.
[reply]

      Hi runrig,

      Sorry - I don't follow.  What I mean by "centralised" is, putting them in a package, so that in general I'll just have a single command for each query in my main scripts.  It can get quite lengthy to do everything in the main scripts, especially when one includes writing the SQL command to a debug logfile.  Is that what you thought I meant?  If so, how do you think connect_cached and prepare_cached would help achieve that?  From looking at them in the DBI doco, it looks as if they are mainly for improving performance on some kinds of repeated connections/queries.

      Thanks.

[reply]
        If you are repeating your queries (and that's what I thought you were trying to do because it's the only reason I could think of to try to share statement handles), then prepare_cached would help. If you want all calls to connect to get the same database handle, then connect_cached might help.

        And on another note, I have a suspicion that you do not Use strict and warnings, which you should be doing.

[reply]

Back to Seekers of Perl Wisdom