反馈、关于我的模块的问题以及我是否应该更改任何内容？

Question

package My::Module;

# $Id$

use strict;
use Carp;
use Data::Dumper;
use DBI;

$My::Module::VERSION  = '0.1';

sub new {
    my ($class, %opt) = @_;
    my $opt_count = keys %opt;

    $class->set_error('');
    #return $class->set_error("Too many arguments to initialize.") if ($opt_count > 5);
    #return $class->set_error("Missing arguments to initialize.") if ($opt_count < 2);

    my $self = bless {
                      _DRIVER_OPTIONS  => $opt{'mysql'},
                     },$class;

    if (not defined $self) {
        return $class->set_error( "new() failed: " . $class->errstr );
    }

    if ($self->{_DRIVER_OPTIONS}->{Host} ne '') {
        $self->{_DRIVER_OPTIONS}->{DataSource} = 'DBI:mysql:database=' . $self->{_DRIVER_OPTIONS}->{Database} . ';host=' . $self->{_DRIVER_OPTIONS}->{Host};
    } else {
        $self->{_DRIVER_OPTIONS}->{DataSource} = 'DBI:mysql:database=' . $self->{_DRIVER_OPTIONS}->{Database} . ';';
    }
    $self->{Handle} = DBI->connect($self->{_DRIVER_OPTIONS}->{DataSource},
                                   $self->{_DRIVER_OPTIONS}->{Username},
                                   $self->{_DRIVER_OPTIONS}->{Password},
                                   { RaiseError=>1, PrintError=>1, AutoCommit=>1 }
                                  );
    return $self->set_error("new(): couldn't connect to database: " . DBI->errstr) unless ($self->{Handle});
    $self->{_disconnect} = 1;

    print Dumper \$self;

    return $self;
}

sub database {
    my $self = shift;
    if (@_) { $self->{Handle} = shift }
    return $self->{Handle};
}

sub set_error {
    my $class   = shift;
    my $message = shift;
    $class = ref($class) || $class;
    no strict 'refs';
    ${ "$class\::errstr" } = sprintf($message || "", @_);
    return;
}

*error = \&errstr;
sub errstr {
    my $class = shift;
    $class = ref( $class ) || $class;

    no strict 'refs';
    return ${ "$class\::errstr" } || '';
}

sub DESTROY {
    my $self = shift;

    unless (defined $self->{Handle} && $self->{Handle}->ping) {
        $self->set_error(__PACKAGE__ . '::DESTROY(). Database handle has gone away');
        return;
    }

    unless ($self->{Handle}->{AutoCommit}) {
        $self->{Handle}->commit;
    }

    if ($self->{_disconnect}) {
        $self->{Handle}->disconnect;
    }
}

1;

1. 这是正确的方法，所以我可以 在我的代码上重用数据库 而不必打开一个新的 连接或也将打开 每次我使用它时都有一个新连接 ?

2. 我是否应该更改 模块 ？还是我做错了什么？

目前我只是在学习并考虑做自己的引擎模块，所以我从这个开始。

简单的测试代码（下面的代码不做评论，只是一个如何使用模块的示例）：

#!/usr/bin/perl

use warnings;
use strict;
use Data::Dumper;
use lib 'path to module';
use My::Module;

my $session = My::Module->new(mysql     => {
                                            Database =>'module',
                                            Host     =>'10.0.0.2',
                                            Username =>'module',
                                            Password =>'module'
                                           }) or die My::Module->errstr;

my $dbh = $session->database();
my $sth = $dbh->prepare(q{
             SELECT session_id
             FROM sessions
          });
   $sth->execute() || die print($dbh->errstr);
my $ref = $sth->fetchall_arrayref({});
$sth->finish;

print Dumper \$ref;

Answer 1

我建议使用现有的数据库接口，而不是自己滚动，因为有许多其他人花费数年时间为您找出和解决的秘密问题。 DBIx::Connector 非常棒，它的fixup 模式可以让你重用数据库连接，甚至跨进程分支。

此外，如果您使用Moose，您将不再需要编写自己的对象构造函数或对象字段。 :)

DBIx::Class 与 Moose 结合使用会更好，但在您发现自己需要更多 ORM-ish 功能之前不是必需的。

Answer 2

除了使用 CPAN 模块来完成这项任务，以下是我的实用建议：

1. 不要从构造函数返回错误值。而是抛出异常。
2. 使用访问器访问类的内部，而不是使用直接哈希访问。

3. 如果您班级的用户没有启用AutoCommit，她选择不启用AutoCommit 是有原因的。因此不要这样做：

   unless ($self->{Handle}->{AutoCommit}) {
       $self->{Handle}->commit;
   }
   

   在DESTROY.

4. 请注意，bless 不会失败，只要给它一个可修改的引用（将其与 open 的行为进行比较，即使 open 的参数也可能无法打开文件是一个有效的文件名，并且会通过返回一个错误值来指示这种情况）。因此，检查bless 的返回值没有任何用处。如果您想处理bless 失败的可能性，则必须捕获致命的运行时异常。

Answer 3

您揭露错误的方式非常、非常过时。如果发生异常情况，为什么不提出适当的异常呢？您似乎在 DBI 模块之后对错误处理进行了建模。请注意，DBI 也有一个 RaiseError 选项。使用它几乎总是比使用老式的errorstr 版本更合理。不幸的是，DBI 现在不能再改变它的默认值了，但是对于新代码，我完全看不出复制这个有缺陷的想法的理由。

您还将根据用户从外部提供的参数在代码中构建 DBI 连接。你这样做有充分的理由吗？允许用户传入他自己构建的DBI::dh 会更加灵活。是的，这需要更多的外部代码来设置对象并将它们连接在一起，但这也会导致更简洁的设计。如果手动连接您的对象太麻烦您，您可能想看看Bread::Board 为您完成连接，而不是影响您的模块设计。

另外，我赞同 Ether 关于使用 DBIx::Connector 的建议。管理数据库句柄真的很费劲，这很容易出错。