I have two answers, one roughly sticks to your framework and the other changes the structure of the code but is how I would divide the problem.

The first version changes your execsql code to take a list of bind params too.

my $sql = 'select comp_name from company where comp_name = ?';

my $query = execsql($sql, $hash{comp_id});

sub execsql {
    my ($sql, @bind_params) = @_;

    my $sth = $dbh->prepare($sql) || error('prep_sql', $0);
    $sth->execute(@bind_params)   || error('sql', $0);
    $dbh->commit;
    return $sth;
}
[download]

The advantages are that it now handles escaping special SQL characters in your variables and putting the right delimiters around strings. The disadvantages are that you are doing more copying but you can get around that if you want by passing references.

What I would do if I were coding this from scratch and had free reign is to separate the DB access from the main application logic so that all SQL queries are hidden in modules (or at least subroutines) in case I ever needed to port to a different DB and the syntax needs to change. So:

my $dbh = get_db_handle();
my $company_name = get_company_name(company_id => $comp_id,
                                    dbh        => $dbh,
                                );
$dbh->disconnect;


sub get_db_handle {
    # Do whatever you need to get the DB handle and set
    # your favorite options
    my $dbh = DBI->connect(...);
    error('DB connect') unless defined $dbh;
}

sub get_company_name {
    my %args = @_;
    my $dbh = $args{dbh};

    my $statement = 'select comp_name from company where comp_name = ?
+';

    my $sth = $dbh->prepare($sql) || error('prep_sql', $0);
    $sth->execute(@bind_params)   || error('sql', $0);
    my ($company_name) = $sth->fetchrow_array;
    $sth->finish;

    return $company_name;
}
[download]

You could certainly use the execsql code in each access subroutine. I prefer not to commit the DB handle inside the accessor functions so that I can call multiple accessors with the same handle and commit all of them as one transaction. In this case it does not matter since I am simply doing a select.

Hope that helped.

-ben

Great comment, but what if I want to use the routine several times in the script using different variables?

qball~"I have node idea?!"

simply recall the execute() function with each different variable - you are not limited to using only the listed variable - you could even do it without a variable at all as in: execute("1"); Only by putting the variable in the $sql definition do you limite yourself to having to "re-prepare" the statement for every value you want to check.

-Adam Stanley
Nethosters, Inc.

Forgot to mention one thing - you need to change the prepare function to prepare_cached otherwise the ? will cause it to fail.

-Adam Stanley
Nethosters, Inc.

Nah, prepare is fine. The prepare_cache just saves the corresponding $sth. This may be faster depending on the DB, but as the docs note it may be risky.

-ben

my $comp_name = get_comp_name($hash{comp_id});

BEGIN {
 my $sth;

 sub get_comp_name {
      my $comp_name = shift;
      $sth = $dbh->prepare("select comp_name from company where comp_n
+ame = ?") unless $sth;
      my $r_name = $sth->selectrow_array($sth, undef, $comp_name);
      return $r_name;
  }
}
[download]

then build a hash with the keys being the placeholder names ( minus the colon ), and the values being the values to use.

%bound_params = ( DATA => 'foo bar'
          MORE => 'bazqux'
        );
[download]

if your DB supports it, used named placeholders ( like :DATA )

Why not learn a db-independent method like the "?" place-holders described above?

there's no reason to not know both ways of attacking the situation. named parameters ( even placeholders ) make code easier to read, and therefore to maintain.

if you don't want to do it, no one is forcing anything.

execute called with 1 bind variables when 0 are needed
[download]

In your $sql definition make sure you have not quoted the ?. It should look like this: select ... from ... where ... = ?

-Adam Stanley
Nethosters, Inc.