Обратная связь, вопрос о моем модуле и если я должен что-то изменить

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.

Answer 2

Кроме использования модуля CPAN для выполнения этой задачи, вот мои практические рекомендации:

1. Не возвращайте значение ошибки из конструктора. Вместо этого выведите исключение.
2. Получите доступ к внутренним компонентам вашего класса, используя средства доступа, а не прямой доступ к хешу.

3. Если пользователь вашего класса не включил AutoCommit, он решил не включать AutoCommit по причине. Поэтому не делайте:

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

   в DESTROY.

4. Обратите внимание, что благослови не потерпит неудачу, пока ему дается модифицируемая ссылка (сравните это, скажем, с поведением open, которое может не открыть файл, даже если аргумент open является допустимым именем файла и будет указывать на эту ситуацию, возвращая ложное значение). Поэтому проверка возвращаемого значения bless не служит какой-либо полезной цели. Если вы хотите обработать возможность сбоя bless, вам придется перехватывать фатальные исключения времени выполнения.

Answer 3

Ваш способ выявления ошибок очень, очень старомоден. Если происходит что-то исключительное, почему бы не создать правильное исключение? Похоже, вы смоделировали обработку ошибок после модуля DBI. Обратите внимание, что DBI также имеет опцию RaiseError. Использование этого почти всегда более разумно, чем использование устаревшей версии errorstr. К сожалению, DBI больше не может изменять настройки по умолчанию, но для нового кода я не вижу смысла копировать эту ошибочную идею.

Вы также создаете соединение DBI в своем коде на основе параметров, предоставленных пользователем извне. У вас есть веская причина для этого? Разрешение пользователю передать значение DBI::dh, которое он сконструировал сам, будет более гибким. Да, это требует немного больше кода снаружи, чтобы настроить объекты и соединить их вместе, но это также приведет к более чистому дизайну. Если подключение ваших объектов слишком вас беспокоит, вы, возможно, захотите взглянуть на Bread::Board, чтобы выполнить подключение за вас, вместо того чтобы идти на компромисс с дизайном вашего модуля.

Кроме того, я повторяю предложение Эфира об использовании DBIx::Connector. Управление дескрипторами базы данных действительно требует больших усилий, что может привести к ошибкам.