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

This week on www.perl.com, there is a program that needs to perform the following task:

 $_ = <INFO> until !defined($_) || /^(\* Menu:|\037)/;
 return %header if !defined($_) || /^\037/;

[download]
It's reading through a file and it wants to exit the read loop on any of three conditions: At end-of-file; at a line that begins with \037, or at a line that begins with * Menu:. In the first two cases the function should return immediately, but if it sees * Menu: it should continue to the next block of code, which reads and processes the following menu.

I don't really like the code above, because it seems redundant. The two pattern matches are similar but not identical, and the test for defined($_) is repeated. Usually in such cases there's a way to rewrite the block without repetition. I messed around with this a while, but wasn't able to come up with anything I was really happy with.

Here's one possibility: 

while (<INFO>) {
    last if /^\* Menu:/;
    return %header if /^\037/;
}
return %header unless defined $_;

[download]
This avoids the repeated regex, but the defined test is still repeated (although implicitly) and the return is also repeated. I kept wishing there was some way to arrange it so that there was only one return.

Simon Cozens suggested this: 

do { 
   $_ = <INFO>; 
   return %header if /^\037/ || ! defined $_ 
} until /^\* Menu:/ ;

[download]
which is better in some ways, but now you have to assign to $_ explicitly, and also it uses do...until which is an unusual construction.

I keep feeling like there is something obvious I am missing. Does anyone have any other thoughts?

Replies are listed 'Best First'.
RE: Control Flow Puzzle
by japhy (Canon) on Nov 16, 2000 at 02:34 UTC
    Would a test for eof() be helpful here: 
    while (<INFO>) {
  last if /^\* Menu:/;
  return %header if /^\037/ or eof(INFO);
}

    [download]


    $_="goto+F.print+chop;\n=yhpaj";F1:eval
[reply]
[d/l]
      Yes, that does look good. Thanks!

      In the article I complained bitterly about eof() and strongly advised staying away from it. But it looks like this is one of the few times when eof() is the Right Thing.

      I guess I was just being too doctrinaire. Well, that'll give me something interesting to say in the followup article.

[reply]
RE: Control Flow Puzzle
by merlyn (Sage) on Nov 16, 2000 at 02:29 UTC
    I tend to break these down to a naked block: 
    {
  $_ = <INFO>;
  return %Header unless defined $_ and !/^\037/;
  last if /^\* Menu:/;
  redo;
}

    [download]
    And as I'm writing this, I'm wondering if something belongs between the last and redo lines there. What are you doing for non 37, non menu lines? Just skipping them?

    -- Randal L. Schwartz, Perl hacker

[reply]
[d/l]
      Randal said: 
      {
   $_ = <INFO>;
   return %Header unless defined $_ and !/^\037/;
   last if /^\* Menu:/;
   redo;
}

      [download]
      That's really interesting. Why do you prefer that to this: 
      while (1) {
   $_ = <INFO>;
   return %Header unless defined $_ and !/^\037/;
   last if /^\* Menu:/;
}

      [download]
      This seems the same to me, only less abnormal.

      > What are you doing for non 37, non menu lines? Just skipping them?
      Yes, exactly so. The 37 line marks the end of the 'node'. If the function sees a 'menu' line before the 37 line, it knows that the node contains a menu, and the following code processes the menu up to the following 37 line. Lines that look different are not of interest to this function.

[reply]
[d/l]
[select]
        Why the naked block instead of the infinite loop? Style thing, I guess. Over time, I usually end up hacking the naked block, adding and deleting conditions of restart and exit. The naked block has cleaner semantics for me, going up only when and where I tell it, and falling out otherwise. Certainly for this case I could have switched to an infinite loop once I saw that the form fits, but I don't know that when I first start, and I consider a naked block to be a more primitive form. {grin}

        -- Randal L. Schwartz, Perl hacker

[reply]
        What about a loop-less solution? 

        undef $/;
$_=<>;
s/(\n\037.*?\n\* Menu.*?\n\037)//;
$mymenu=$1;

        [download]
        This way you put all input into one string, and just pick out the menus one by one. No tricky loops anymore. I don't know how much you rely on the markers to start at the beginning of the line, but you could remove the newlines in the regex, of course.

        I pray I didn't say something very stupid here. Please excuse me if this is the case..... And credits to little, who only yesterday showed me the undef $/ trick.

        Cheers,

        Jeroen

        Post Posting: Mr. Schwartz, `Learning perl' was my first contact with perl, and I really loved it, and enjoyed reading it very much! So, thanks a lot...

        I was dreaming of guitarnotes that would irritate an executive kind of guy (FZ)

[reply]
[d/l]
(Ovid) Re: Control Flow Puzzle
by Ovid (Cardinal) on Nov 16, 2000 at 02:40 UTC
    This seems fairly straightforward: 
    while (<INFO>) {
    return %header if /^\037/ or eof( INFO );
    last if /^(\* Menu:)/;
}

    [download]
    D'oh! Looks like japhy beat me to it!
[reply]
[d/l]
Re: Control Flow Puzzle
by runrig (Abbot) on Nov 16, 2000 at 03:06 UTC
    I don't think anyone's suggested this yet: 
    $_ = <INFO> until !defined($_) || /^(\* Menu:|\037)/;
return %header unless /Menu/;

    [download]
    Although that spits out warnings under -w, so maybe: 
    $_ = <INFO> until !defined($_) || /^(\* Menu:|\037)/;
return %header unless defined and /Menu/;

    [download]
    and you don't really need the second regex so maybe: 
    $_ = <INFO> until !defined($_) || /^(\* Menu:|\037)/;
return %header unless defined and index($_, 'Menu') >= 0;

    [download]
    Or if you want to split the regex: 
    my $menu;
$_ = <INFO> until !defined($_) || ($menu = /^\* Menu:/) || /^\037/;
return %header unless $menu;

    [download]
    And then of course you could use substr() instead of a regex for the \037 test...

    Oops just realized that the 1st, 2nd, and 3rd solutions presuppose that you don't have 'Menu' AFTER the \037 (which could be fixed with a '^ *' in the 1st & 2nd answers)...
[reply]
[d/l]
[select]
Re: Control Flow Puzzle
by Dominus (Parson) on Nov 30, 2000 at 06:50 UTC
[reply]
      For a source of appropriate code, I suggest wandering through Snippets, Code, and Craft then asking the appropriate monk.

      Also I noticed that you haven't been using strict in the series yet. Tsk, tsk. :-)

[reply]
        'strict' is overrated.
[reply]

Back to Seekers of Perl Wisdom