coldfingertips has asked for the wisdom of the Perl Monks concerning the following question:

This is in relation to 325952.

Still having problems and I have made a few improvements.

What this snippet is doing is tying a hash to my database %upload which contains image information. When images are uploaded to my gallery, I give them $key++ and I compare key names to print them in the correct order.

Now I have a delete function on my script where I delete a key to remove all the images and it's information from my database. It seems to work fine if I delete most images, everything else works. But when I delete the newest image (no matter what number it is, it seems to delete more than one image and decides to remove a few other ones.

In short, when images are uploaded through a form I do a num++ on their key. This means I have keys 0-how many ever images I have at the time. When I delete a key that's somewhere in between, let's say I want to delete key 14 when I have 20 total.. I need it to rewrite the database so there are keys 0-19 without a gap in key numbers and have all the data still be accurate.

Hard to explain why I need to remove the gap, but it's something I need it to do. Can anyone see my logic mistake?

Thank you for your help! 

sub processing
{
my %upload;
my $upload = "gal.db";

tie %upload, "DB_File", "$gal", O_CREAT | O_RDWR, 0644, $DB_BTREE
  or die "Cannot open file 'gal': $!\n";

###
# Setup our params
###
my $edit = url_param('edit');
my $del  = url_param('rem');

####################
# Check for DELETE call
####################
if ($del ne "")
{
  if (exists $upload{$del})
  {
    my ( $filename, $title, $comments, $width, $height, $count ) = spl
+it ( /::/, $upload{$del} );
    
    ###
    # Rewriting $filename to a .png
    ###
    my $thumbs = $filename;
    $thumbs    =~ m/([a-zA-Z0-9]+)\.()/;
    $thumbs    = "$1.png";

    ###
    # Deleting files from both image folders
    ###
    delete $upload{$del};
    unlink "$fullimagedir/$filename" or die "Cannot delete image: $!";
    unlink "$fullthumbsdir/$thumbs" or die "Cannot delete image: $!";
    print "<center><font color=blue>Image removed.</font></center>";  


my $num = "-1";
foreach (sort keys %upload)
{
     $num++;
     my ( $filename2, $title2, $comments2, $width2, $height2, $count2 
+) = split ( /::/, $upload{$_} );
     my $joined = join("::", $filename2, $title2, $comments2, $width2,
+ $height2, $count2);
     $upload{$num} = $joined;
     print "$num = $joined<br>";
}

my $count = "-1";
foreach (sort keys %upload)
{
   $count++;
}

print "$count";
delete $upload{$count};

  }
  else {
     print "<center><font color=red>ERROR!  Image does not exist.</fon
+t></center>";
  }

[download]

update (broquaint): added <readmore>

Replies are listed 'Best First'.
Re: rebuilding a hash (more)
by ysth (Canon) on Feb 20, 2004 at 07:58 UTC
    The sort keys not doing a numeric sort is definitely going to cause some unusual effects for you. The other very wrong thing I see is that second delete $upload{count}. I think that will cause you to delete an extra record when you are trying to delete the last one. Fix it by skipping the second loop (everything that involves $count) and just deleting $upload{++$num} after the first loop instead.

    In terms of style points, you should be saying = -1, not = "-1"; there's no point in setting a string and then turning around and incrementing it; just use a number.

    I assume you are using the split-up parts $filename2, etc. in code that you've trimmed away; if these are really unused, get rid of the split and join lines and just use $upload{$_} where you have $joined.

    And I'd say $upload{$num} = $joined if $num != $_; no point in updating your database if nothing changed.

[reply]
[d/l]
[select]
      Thank you so much!! It now rebuilds the hash without gaps and I can delete the first, last or middle images and have it work perfectly. Though I don't see why removing the second foreach did the trick (I only deleted one time), but it works like a charm.

      Thanks for all your help!

[reply]
        The second foreach would always delete the last thing in the hash (where last thing means after the first delete). This works where you've moved everything up to close up a gap, since the last entry would be an invalid one then. It doesn't work where you've deleted the last entry; it goes ahead and deletes what was originally the second to last entry.

        Try writing down on paper what happens at each step, with keys 1, 2, and 3 and deleting key 3.

[reply]
Re: rebuilding a hash (more)
by Skeeve (Parson) on Feb 20, 2004 at 06:51 UTC
    Besides the fact, that your code is quite complicated, you have this in it:
    foreach (sort keys %upload)

    [download]
    sort will sort alphabetically giving you (example):
    0
1
10
100
101
11
:
19
2
20
:

    [download]
    If you use numbers as indices, you should use an array!

    otherwise try sort { $a<=>$b } keys %upload
[reply]
[d/l]
[select]
      If you use numbers as indices, you should use an array!

      Not necessarily. It makes no sense to use an array if the indices of its only elements are 2, 24, and 35000. For sparse arrays, hashes are a good choice.

      As for his sort problem, there are ways around it by using a smarter sort function.


      Dave

[reply]
        > Not necessarily.

        Agreed. But as he said, he don't want to have a gap, and his code is (at least) trying to close gaps, arrays would be, to my understanding, the best choice here.
[reply]
      I replaced it with the correct sort, I overlooked that. But does anyone know why I it appears I can delete images correctly but when I try to delete the newest one it deletes a few images?
[reply]
Re: rebuilding a hash (more)
by tbone1 (Monsignor) on Feb 20, 2004 at 13:36 UTC

    One minor, niggling thing: wouldn't it be better to use ++$key instead of $key++? ++$key iterates before assignment, whereas $key++ iterates afterwards. I can imagine situations where either one would be preferable, and maybe it makes no difference since you are using it as a key to a hash.

    Just my $.02.

    --
    tbone1, YAPS (Yet Another Perl Schlub)
    And remember, if he succeeds, so what.
    - Chick McGee

[reply]

Back to Seekers of Perl Wisdom