Beefy Boxes and Bandwidth Generously Provided by pair Networks
We don't bite newbies here... much

Nested subs - inherently evil?

by myocom (Deacon)
on Feb 11, 2002 at 20:01 UTC ( #144688=perlquestion: print w/replies, xml ) Need Help??
myocom has asked for the wisdom of the Perl Monks concerning the following question:

I'm writing a script that has to read from a binary file. Sometimes it has to read 1 byte, sometimes 4, sometimes 12. This seems ripe for a function, so I wrote one:

sub ReadBytes { my $num = shift; return undef if $num < 1; my @bytes; for (1..$num) { my $byte; read(BINFILE, $byte, 1); push @bytes, unpack("C",$byte); } return @bytes; }

The problem I have is that while I will always be reading from BINFILE in *this* script, this sub isn't very flexible and doesn't lend itself to code re-use (and therefore offends my sense of aesthetics). Further, since BINFILE is opened within another sub (ParseData, which does interesting things to the bytes I'm reading from BINFILE), I have ReadBytes embedded in ParseData. Nested subs are icky, in my book (there's that darned sense of aesthetics again), and there must be a better way to do it.

So my question is this: Just what is the Better Way To Do It?

Update: Passing the filehandle is just what I was looking for. For some reason I thought I'd have to tie it in some way - I forgot about passing a ref to the typeglob. Thanks all for your help!

"One word of warning: if you meet a bunch of Perl programmers on the bus or something, don't look them in the eye. They've been known to try to convert the young into Perl monks." - Frank Willison

Replies are listed 'Best First'.
(Ovid) Re: Nested subs - inherently evil?
by Ovid (Cardinal) on Feb 11, 2002 at 20:22 UTC

    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; }

    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; }

    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 ) };

    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; }


    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.

Re: Nested subs - inherently evil?
by rjray (Chaplain) on Feb 11, 2002 at 20:41 UTC

    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); }

    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.


      I'd use $/ instead of read() so that I didn't have to use a temporary buffer.
      local $/ = \$num; return unpack "C*", scalar <$fh>;

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

Re: Nested subs - inherently evil?
by ejf (Hermit) on Feb 11, 2002 at 20:38 UTC

    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.

Re: Nested subs - inherently evil?
by traveler (Parson) on Feb 11, 2002 at 20:42 UTC
    Ignoring error handling, I'd probably do it something like this:
    sub ReadBytes { local $file = shift; my $num = shift; my $input; read($file, $input, $num); return unpack("C$num", $input); }
    Note that the filehandle is "local" and not "my". Call this as ReadBytes(*BINFILE, 12); You could also localize a typeglob and then use the filehandle directly.

    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, ... }
        I realized it was more straightforward to use $whatever, but neglected to change the local to a my.


Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://144688]
Approved by root
and all is quiet...

How do I use this? | Other CB clients
Other Users?
Others about the Monastery: (5)
As of 2017-03-24 05:11 GMT
Find Nodes?
    Voting Booth?
    Should Pluto Get Its Planethood Back?

    Results (296 votes). Check out past polls.