Don't hardcode the classname. This will break inheritance, if you ever need it. Also, you weren't using strict in your package. Shame on you! :)
package db_dave;
use strict;
use DBI;
use vars qw/ @ISA /;
@ISA = qw/DBI/;
sub new
{
my $class = shift;
my $dbh = DBI->connect('dbi:Oracle:foo', 'foo', 'foo', { RaiseErro
+r => 1 })
|| die "Cant' connect: $DBI::errstr";
my $data = {
_dbh => $dbh
};
bless $data, $class;
return $data;
}
sub runsql_returnString_NEW
{
my ($self, $q, $err) = @_;
my $dbh = $self->{ _dbh };
my $sth = $dbh->prepare($q);
$sth->execute || &error($q, $err); # <- why do you have a regular
+function in an OO module?
my $returnstring;
while (my @array = $sth->fetchrow) {
# ?? You're overwriting this every time :(
# I can't tell what type of data structure you want
$returnstring = $array[0];
}
$sth->finish;
return $returnstring;
}
That should give you a start (though the code is untested). It needs a lot of work.
Cheers,
Ovid
Join the Perlmonks Setiathome Group or just click on the the link and check out our stats. | [reply] [d/l] |
Thanks Ovid - you've given me some great perl nuggets that helped really explain how to work object oriented coding in perl! I figured since this was so helpful to me, I'd post my code, fixed up, since you posted on this thread, maybe this can help someone else. (I personally am going to use this stuff for quick and dirty data cleanup tasks...)
You guys were right about reinventing the wheel, I'm sure there are much better modules out there that would be better for maintainability's sake - I just wanted to do this as an exercise and got exactly what I wanted!
Thanks again...
package db_dave;
use strict;
use DBI;
use vars qw/ @ISA /;
@ISA = qw/DBI/;
sub new
{
my $class = shift;
my ($db, $username, $password) = @_;
if ($password eq "") {
print STDOUT "## enter password for " . $username . "\@" . $db
+ . ": ";
$password = <STDIN>;
chomp $password;
if ($password eq "") {die;}
}
my $dbh = DBI->connect("dbi:Oracle:" . $db, $username, $password,
+{ RaiseError => 1 }) || die "Cant' connect: $DBI::errstr";
my $data = { _dbh => $dbh };
bless $data, $class;
print STDERR "\n## connected \n";
return $data;
}
sub disconnect
{
my ($self, $q, $err) = @_;
my $dbh = $self->{ _dbh };
$dbh->disconnect;
print STDERR "\n## disconnected \n\n";
return;
}
sub runsql_string
{
my ($self, $q, $err) = @_;
my $dbh = $self->{ _dbh };
my $sth = $dbh->prepare($q);
print STDERR "\n## runsql_string: $q \n";
$sth->execute || die;
my $returnstring;
my @array;
while (@array = $sth->fetchrow) { $returnstring = $array[0];}
my $stringcount = @array;
if ($stringcount > 1) { die; }
$sth->finish;
if ($::DEBUG) { print STDERR "[0] " . $returnstring . "\n"; }
return $returnstring;
}
=begin testing
use strict;
use DBI;
use lib "./";
use db_dave;
$::DEBUG = "1"; # prints query results
my $dbh = new db_dave("foo", "foo", "foo");
my $err = "error";
$q = "select count(*) from table";
$dbh->runsql_string($q, $err) . "\n";
$dbh->disconnect;
=end testing
-mr.dunstan | [reply] [d/l] |
- If you are doing this for a learning experience, there are plenty of examples in the SQL and DBIx hierarchies on CPAN. Download a few and look at what they did.
- If you are trying to make a contribution to CPAN with the idea that what you are doing is somehow new, unique, or fills a gap, please be sure to look at the above and also heed the recent words of Dave Rolsky on the dbi-users@perl.org mailing list (which you should join if you are serious about staying up to date on such issues)
There are a lot of tools that already provide wrappers around DBI. These
include:
Alzabo, DBIx::RecordSet, and Class::DBI all provide fairly high level
RDBMS-OO mappers.
Other lower-level wrappers include DBIx::Abstract, DBIx::Easy,
DBIx::SearchBuilder, and EZDBI.
There's really more than enough of these things at this point. Probably
one of them already does most of the things yours does, so I'd suggest you
find that one and offer the author patches for any features you think it
might be missing.
| [reply] |
DOH! I am so embarrassed. Of course I use strict always, this was slapped into a new file from some old code so I was going to tighten the bolts later.
Thanks for all the useful feedback, the regular function was getting called from the same package so that's why I was doing that ...
... yes, I know I am obnoxiously using the wrong DBI method to return a string (I am only expecting one row back, I have other functions to handle arrays, hashes, AoA's etc.. )
Thanks for the inheritance tip, that's why I'm getting into fixing this old stuff.
As far as CPAN goes, all that stuff is useful, sure, but it's not doing things -my- way!! :) I leave the glory for all you guys ...
-mr.dunstan
| [reply] |
| [reply] |