Nested subs aren't. Both subs wind up with a slot in the current package's symbol table. The following two examples are more or less equivalent:

Example 1:

sub foo
{
    my $foo_arg = shift;
    my $foo_return = bar( $foo_arg );
    sub bar
    {
        my $bar_arg = shift;
        $bar_return = munge_munge( $bar_arg );
        return $bar_return;
    }
    return $foo_return;
}
[download]

Example 2:

sub foo
{
    my $foo_arg = shift;
    my $foo_return = bar( $foo_arg );
    return $foo_return;
}

sub bar
{
    my $bar_arg = shift;
    $bar_return = munge_munge( $bar_arg );
    return $bar_return;
}
[download]

I know that structurally, they look different, but they aren't. That's because subs are global in nature and cannot be lexically scoped. They do have one subtle difference, though: they can throw strange "variable will not stay shared" errors if the inner sub tries to access lexically scoped variables. If, for some reason, you feel the need to do this, you want a reference to a sub:

my $sub_ref = sub { munge_munge( shift ) };
[download]

Then, you can lexically scope the sub reference and you won't have a problem. Whether or not that's appropriate in this case I will leave to wiser monks than I. However, I will point out that if you really want that sub to be more flexible, pass in the filehandle. (The following is untested)

open BINFILE, "<", "somefile.txt" or die $!;
ReadBytes( $num, \*BINFILE );

sub ReadBytes {
    my ( $num, $binfile ) = @_;
    return undef if $num < 1;
    my @bytes;
    for (1..$num) {
        my $byte;
        read($binfile, $byte, 1);
        push @bytes, unpack("C",$byte);
    }
    return @bytes;
}
[download]

Cheers,
Ovid

Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

While glob-refs were all the rage, I'd prefer to be fashion-concious and use IO::File instead. :-)

------
We are the carpenters and bricklayers of the Information Age.

Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement.

A good way to approach this is to pass in the filehandle as one of the parameters. You can pass it from the caller either using the syntax the previous reply used, or by using the IO::File class to manage your open filehandles. As for the subroutine itself:

sub ReadBytes {
    my $fh  = shift;
    my $num = shift || 1; # Default to 1 byte instead of error
    return undef unless ($fh);

    my $bytes;
    return undef unless (read($fh, $bytes, $num));

    unpack("C$num", $bytes);
}
[download]

Note that you don't need the for-loop, since both read() and unpack() can use the byte-count you already have. This routine isn't very helpful to the caller in error cases, you may want to do something cleaner with the two points where undef is being returned.

--rjray

I'd use $/ instead of read() so that I didn't have to use a temporary buffer.

local $/ = \$num;
return unpack "C*", scalar <$fh>;
[download]

_____________________________________________________
Jeff[japhy]Pinyan: Perl, regex, and perl hacker, who could use a job
s++=END;++y(;-P)}y js++=;shajsj<++y(p-q)}?print:??;

While I'm not sure what exactly you are asking for, I'm going to try to answer anyway. ;-)

First off, nested subs aren't neccesarily evil. In a way, a class/module is nothing more than specially nested subs. If it looks right, it usually is.

As to hardcoding BINFILE into your sub : why ? You could just pass the filehandle as an argument to ReadBytes as well. You could even use objects (see IO::File) if you're uncomfortable passing fileglobs around ...

You could also eliminate ReadBytes completely ... a

read(BINFILE, $_, 4) && map (unpack("C", $_), split(//))

will yield the same as ReadBytes -- depending on how often you use it, this might be preferable (or maybe you can use it to shorten the ReadBytes sub into a oneliner ;) These functions are all discussed in perlman:perlfunc, amongst others ...

As I don't know what ParseData will ultimately do with these bytes, it's difficult to optimize these routines further -- maybe you don't even need the bytes separated into lists of integerized chars ... ;)

update : I forgot completely about unpack -- as seen in traveler's post, a read(BINFILE, $_, 4) && unpack ("C4", $_) would do just as nicely. I'm not sure, but even unpack ("C".read(BINFILE, $_, 4), $_) might work if you're sure there's enough data left to be read.

sub ReadBytes {
  local $file = shift;
  my $num = shift;
  my $input;
  read($file, $input, $num);
  return unpack("C$num", $input);
}
[download]

HTH, --traveler

Note that the filehandle is "local" and not "my".

What is the purpose of this?

Localizing a typeglob (let us be accurate here: what we call a "filehandle" is a typeglob from which Perl automagically extracts the IO slot) temporarily makes a different referent available by the same name. This is handy for avoiding conflicts elsewhere.

The alternative to your construct, using a lexical to contain a glob reference is also automagically understood by Perl and is, of course, lexically scoped.

At the risk of severe understatement, local is designed to allow scoped modifications of global names. It produces no special magic for filehandles (with the appropriate understanding that there is severe magic in perlio that doesn't apply here).

In other words, you can do it this way, but I can't see where it does you any good, since the standard approach does the same thing more idiomatically.

This results from my initially localizing the glob itself as in:

sub foo {
  local *FH = shift;
...
  read FH, ...
}
[download]

I realized it was more straightforward to use $whatever, but neglected to change the local to a my.

--traveler