CODEWITHSUNDEEP
perlsqlitedbi
HeavyHead
HeavyHead

Reputation: 330

Perl SQLite DBI INSERT via an Array

I have the following pretty complex Perl code that is fed by a dynamic array varying in size but typically between 10 and 20 rows. I'm therefore running this in a loop but reckon there must be a more elegant way of inserting the data.

Is it possible to improve this code to make it more efficient? 

sub data_fill {

  my $target = shift;
  my (@input_data) = @_;

  # Delete the old data
  my $sth = $dbh->prepare("DELETE FROM work_data WHERE target = '$target' ");
  $sth->execute();

  # Insert the new data
  my $sql       = qq{INSERT INTO work_data (target, trackingno, temp, apple, orange, banana, strawberry, lettuce, apricot, peach, blackberry, melon, lemon) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?)};
  my $sth       = $dbh->prepare($sql);
  my $arraysize = scalar(@input_data);
  my $i         = 0;

  while ($i < $arraysize) {

    $sth->bind_param(1, $target,             SQL_VARCHAR);
    $sth->bind_param(2, 'OK',                SQL_VARCHAR);
    $sth->bind_param(3, $input_data[$i],     SQL_VARCHAR);
    $sth->bind_param(4, $input_data[$i + 1], SQL_VARCHAR);
    $sth->bind_param(5, $input_data[$i + 2], SQL_VARCHAR);
    $sth->bind_param(6, $input_data[$i + 3], SQL_VARCHAR);
    $sth->bind_param(7, $input_data[$i + 4], SQL_VARCHAR);

    my $lettuce = $input_data[$i + 5];
    my $apricot = $input_data[$i + 6];
    my $peach   = $input_data[$i + 7];

    if (length($lettuce) == 0) {
      $lettuce = -1;
    }    # Safety net to help later in db sorting
    if (length($apricot) == 0) {
      $apricot = -1;
    }    # Safety net to help later in db sorting
    if (length($peach) == 0) {
      $peach = -1;
    }    # Safety net to help later in db sorting

    $sth->bind_param(8,  $lettuce,             SQL_INTEGER);
    $sth->bind_param(9,  $apricot,             SQL_INTEGER);
    $sth->bind_param(10, $peach,               SQL_INTEGER);
    $sth->bind_param(11, $input_data[$i + 8],  SQL_VARCHAR);
    $sth->bind_param(12, $input_data[$i + 9],  SQL_VARCHAR);
    $sth->bind_param(13, $input_data[$i + 10], SQL_VARCHAR);
    $sth->execute;

    $i = $i + 11;    # INCREMENT
  }
  $dbh->disconnect;
  return;
}

Upvotes: 1

Views: 794

Answers (1)

Borodin
Borodin

Reputation: 126722

Your code is not so much complex as convoluted.

The first thing I see is that $dbh->prepare is called every time the subroutine executes. Worse still, there is a $dbh->disconnect before the return.

I would also look at the binding, which could be permanent if you copied the parameters to a static array instead of binding them all every time.

Update

This is to try to explain what I mean. The code inside the unless statement will be executed only once - the first time the data_fill subroutine is called. It prepares the delete and insert statements, and binds the @params array to the place holders in the insert statement.

Note that the prepare and bind_param calls don't have to be within this subroutine. They are really part of initialisation, and the unless is there to make sure that they are executed only once.

Once all that has been set up, all that remains is to execute the delete statement, and then repeatedly fill the @params array from the subroutine parameter list and execute the insert statement. I have removed the $dbh->close altogether as it's not the right place to be closing the database handle.

This solution has far too many "magic numbers" in it, and I wouldn't publish it as it stands, but I don't know enough of your requirement totell whether I'm even on the right lines.

Note that none of this is tested, beyond checking that it will compile

my ($delete, $insert, @params);

sub data_fill {

  my $target = shift;
  my (@input_data) = @_;

  unless ($delete and $insert) {

    $delete = $dbh->prepare('DELETE FROM work_data WHERE target = ?');

    $insert = $dbh->prepare('
        INSERT INTO work_data (
          target, trackingno, temp, apple, orange, banana,
          strawberry, lettuce, apricot, peach, blackberry, melon, lemon
        )
        VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
    ');

    for my $i (0 .. 10) {
      my $mode = SQL_VARCHAR;
      $mode = SQL_INTEGER if $i >= 7 and $i <= 9;
      $sth->bind_param($i+1, $params[$i], $mode);
    }
  }

  $delete->execute($target);

  while (@input_data >= 11) {
    my @record = splice @input_data, 0, 11;
    @params = ($target, 'OK', @record);
    for (@params[7..9]) {
      $_ = -1 unless length > 0;
    }
    $insert->execute;
  }
}

Upvotes: 2

Related Questions