BinPack Algorithm Use To Pack Files In a DVD

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

I'm trying to figure out if I have written the proper code for the task I'm trying to accomplish. Anybody willing to help?

#!/usr/bin/perl;
use strict;
use warnings;
use Algorithm::BinPack;

# Defines DVD capacity as a constant in bytes
use constant DVD_CAP => 4707319808;

# Change to directory below
my $dest = "/pas/rentals/pendbackup"; 
chdir $dest;
system("pwd");

# Creates a directory with the current date and
#  stores its value in the scalar variable $dir
my $dir =(mkdir(`date +%Y%m%d`),0644) or print $!;


# Create a new BinPack object and set the binsize
#  value to the DVD capacity constant DVD_CAP
my $binpack = Algorithm::BinPack->new(
    binsize => DVD_CAP
);
print "Got here before adding items!\n\n";

# Open directory $dest to process readdir that returns
#  the directory entries for the directory $dest opened by opendir

# map: evaluates BLOCK {(stat[7])} for each element of LIST (glob '*')
#  and returns the list value of each evaluation (file size).
# stat[7]: identifies the size of files in bytes
# glob: returns a list of filenames determined by EXPR (glob EXPR).

# Add items to the bin with 'label' and 'size' using the loop
# Then it closes the directory $dest
my $filesize;
opendir(DIR,$dest) or die "Can't open directory $dest: $!";
while (my $file = readdir(DIR)) {
    next if $file =~ /^\.\.?$/; # skip . and ..
    $filesize = map {(stat)[7]} glob "*" || die "stat($file): $!\n";
    if (-f $file) {
        $binpack->add_item(label => $file, size => $filesize);
    }
}
closedir(DIR);

# Executes the mkdir function to create the directory
$dir or die "Can't mkdir $dir: $!";

my $packed_bin;
my $item;
for $packed_bin ($binpack->pack_bins) {
    print "Bin size: ", $packed_bin->{size}, "\n";
    for $item (@{ $packed_bin->{items} }) {
        printf " %-6s %-20s\n", $_, $item->{$_} for keys %{ $item };
        print  " ---\n"; 
    }
}
system ("mv -t $packed_bin $dir");
[download]

Please, any help is welcome! Thank you. Marco

Comment on BinPack Algorithm Use To Pack Files In a DVD Download Code

Replies are listed 'Best First'.
Re: BinPack Algorithm Use To Pack Files In a DVD by roboticus (Chancellor) on Apr 14, 2010 at 19:02 UTC
mgrangeiro: You probably ought to specify exactly what the task you're trying to accomplish is. While I could read the code and guess what the exact task is, guessing is unsatisfying. Also, if you rely on the code to be the specification, then obviously the code you provided is the best code that satisfies the requirements. Since we can't see the actual requirements, we can't differentiate between bugs and shortcomings from actual customer requirements. ...roboticus	[reply]
Re: BinPack Algorithm Use To Pack Files In a DVD by GrandFather (Saint) on Apr 14, 2010 at 22:33 UTC
Have you run it? Did it do what you expected? It may help others understand where you are coming from if you reference your previous question (I want to maximize data storage in a DVD) to provide some context. Personally I find the script vastly over commented to the extent that it is hard to find the code. You should also note that the interpreter doesn't read comments so mismatches between the intent indicated in the comment and the actual code (c.f. "# Executes the mkdir function ...") may lead to unhappiness on the part of code maintainers. Always declare loop variables in the loop header: `for my $item (@items) {`. Don't declare loop variables outside the loop header - they are not what you expect! True laziness is hard work	[reply] [d/l]
Re: BinPack Algorithm Use To Pack Files In a DVD by ikegami (Patriarch) on Apr 14, 2010 at 23:35 UTC
As I previously mentioned, files take up more space than their file size. `+----+----+----+----+----+ \| \|XXXX\|XXXX\|XX\| \| +----+----+----+----+----+ \| \| = Disk block XX = File = Wasted space` [download] You need to round up file sizes to the next multiple of a block size. `use POSIX qw( ceil ); $filesize = ceil($filesize / BLOCK_SIZE) * BLOCK_SIZE;` [download] Directories also take up space, but I have no idea how much. Unless you plan on having lots of directories, the simplest solution might be to lower `DVD_CAP` by a bit. I have no idea what's reasonable, but 64k is probably not a bad start. `# Space for directories. No idea what's reasonable. use constant DIR_BUF => 64*1024; use constant DVD_CAP => 4707319808 - DIR_BUF;` [download]	[reply] [d/l] [select]
Re: BinPack Algorithm Use To Pack Files In a DVD by pemungkah (Priest) on Apr 14, 2010 at 23:24 UTC
Let's break it down. We'll assume for the sake of simplicity that Algorithm::BinPack does what it says and that we don't need to test that. So the things you need to test are: Does the mkdir create the directory you want for the output? Does the while loop find all the files in the directory you're backing up? Does the list of files per bin look right? You can't test any of those as the code is currently organized because the logic's all in line. If you wanted to be able to test this (and you should want to; you're depending on it to back up your data!), you'll need to reorganize it so it's testable. It's not necessary to go all the way to a separate package; you can get away with wrapping the functions in subs and then modifying your main-line code to call them. You can then add a way to run a test vs. doing a backup via Getopt::Long or an environment variable. (Personally I'd spend the time to create the package simply because I could leverage Perl's build-and-test infrastructures.) On a cursory scan, I think this `$dir or die "Can't mkdir $dir: $!";` [download] doesn't do what you think it does. If you `qx()` the string in `$dir` then it will be executed, but just referencing it like this, Perl will only check it to see if it's "true" (i.e., not null in this case), and will not execute it! The `while` loop may work, but honestly, you need to test it to be sure that it does. A couple other notes: what happens if one of your files is bigger than a DVD? (Right now you'll attempt to `mv` it and fail.) If the subroutine wrapping seems like too much work, then you should at least create some sample directories to be backed up, figure out by inspection what should happen, and then run the script to see if it does. If you change the size of a DVD to (say) 500 bytes, you can create a few files of a couple hundred bytes each and try it out. EDIT: typos.	[reply] [d/l]