http://qs321.pair.com?node_id=382243

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

Hello fellow monks,

I am working on a little module to scan a whole tree of sourcefiles for use with Vim. One of the methods is:

sub parseDir {
    my $self    = shift;
    my $path    = shift;  # the path to the directroy we are
                          # currently scanning.
    my $out     = shift;  # the file we write the results to
    my $target  = shift;  # the thing we are searching for

    my $dir = IO::Dir->new($path) or die "can't open $path : $!";
    my @files = $dir->read();
    $dir->close();
    # Scan all interesting files
    grep(   ( -f "$path/$_" ) and 
            ( $_ =~ m/\.(java|xml|cpp|c|h|inc|pas)$/i ) and
            ( $self->Scan_File( "$path/$_", $out, $target ) )  , @file
+s );
    # Also scan any subdirectories.
    grep( ( ( -d "$path/$_" ) and 
            ( $_ !~ /^\.+$/ ) and # but avoid . and ..
            ( $self->parseDir( "$path/$_", $out, $target ) ) ), @files
+ );
}

[download]

The odd thing is that the second grep works fine while the first grep results in a: 'Not enough arguments for grep at scan.pl line 63, near "@files )"'. The only difference between the two is that the first has a '(' ')' pair less. I am quite puzzled why this makes a difference, since both

 
  ( expression ) and ( expression ) and ( expression)

[download]

and

 
  ( ( expression ) and ( expression ) and ( expression) )

[download]

seem to me to be syntactically correct expression.

Can anybody shed some light on this?

Have Fun

Replies are listed 'Best First'.
Re: Confusing syntax error with grep
by etcshadow (Priest) on Aug 12, 2004 at 14:15 UTC
    Don't try to futz with 
    grep (expr), @list;

    [download]
    or 
    grep ((expr), @list);

    [download]
    or any such... you just confuse the hell out of the parser and out of someone trying to read it. Instead use this syntax for grep: 
    grep { block } @list;

    [download]
    Those are curly braces setting off a block, not parenthesese setting off an expression. So your code would become: 
        grep {
        ( -f "$path/$_" ) and 
        ( $_ =~ m/\.(java|xml|cpp|c|h|inc|pas)$/i ) and
        ( $self->Scan_File( "$path/$_", $out, $target )
    } @files );
    # Also scan any subdirectories.
    grep {
        ( ( -d "$path/$_" ) and 
          ( $_ !~ /^\.+$/ ) and # but avoid . and ..
          ( $self->parseDir( "$path/$_", $out, $target )
        )
    } @files;

    [download]
    However, it also doesn't make sense because you are using grep in a void context. grep is meant to filter a list (reduce some of the contents out of list1 into list2). If you just want to apply a block of code against every item in a list, think of using map or foreach. Most folks here will complain, likewise, if you use map in a void context (for the same reason, sort of, that I complain about you using grep... the purpose of map (in perl) is to *mutate* a list... although in some other language paradigms one might think of map as making sense in a void context... but that's another argument). 
    ------------
:Wq
Not an editor command: Wq

    [download]
[reply]
[d/l]
[select]
      However, it also doesn't make sense because you are using grep in a void context.

      Not necessarily. The first grep is in void context, but the last one may or may not be -- it depends on how the subroutine is called. My guess is he took some working code and added a second grep at the end, without really realising how it worked.

      Update: It's pointless to guess how it happened. Kindly ignore that sentence. :-)

[reply]
[d/l]

      "However, it also doesn't make sense because you are using grep in a void context. grep is meant to filter a list (reduce some of the contents out of list1 into list2). If you just want to apply a block of code against every item in a list, think of using map or foreach. Most folks here will complain, likewise, if you use map in a void context"

      Guess I agree on using map instead of grep. But I find foreach ugly.

        foreach ( @alist ) {
    if ( condition( $_ ) ) {
      do_something( $_ );
    }
  }

      [download]

      Is far less pretty to the eye than:

         map { condition( $_ ) and do_something( $_ ) } @alist;

      [download]

      OK, map ( or grep ) has to create a list which is not used. I used to worry about that too. However, given the size of an list in an average problem and the amount of memory that is nowadays present in a computer it is no longer relevant to worry about it. For instance in my case the list holds the names of all files in a single directory. That will be 1 to 1000 elements or so. Compared to the 256 Mbyte that is available in my computer this is peanuts. So I stopped worrying about it and instead worry about making it readable.

      Of course it would be even nicer if there was a construct in perl like:

         { do_something( $_) } forall ( @alist ) where { condition( $_ ) }

      [download]

      Have Fun

[reply]
[d/l]
[select]
        Of course it would be even nicer if there was a construct in perl like:    { do_something( $_) } forall ( @alist ) where { condition( $_ ) }
        You mean like: do_something( $_ ) for grep condition( $_ ), @alist; ?

        Update: condition( $_ ) and do_something( $_ ) for @alist; is going to be faster because it doesn't have to loop twice.

[reply]
[d/l]
[select]

        Come to think of it:

        foreach ( @alist ) {
    if ( condition( $_ ) ) {
      do_something( $_ );
    }
}

        [download]

        can be written as

        foreach ( @alist ) { condition( $_ ) and do_something( $_ ) }

        [download]

        Which is as pretty as the map version. The point about worrying too much about memory still stands though.

        Have Fun

[reply]
[d/l]
[select]
Re: Confusing syntax error with grep
by rsteinke (Scribe) on Aug 12, 2004 at 14:06 UTC

    I believe 'and' actually has a lower operator precedence than the comma operator, so your first expression is parsing as 

    ( expression ) and ( expression ) and ( expression, @files )

    [download]
    Try replacing 'and' with '&&' and see if that helps.

    Ron Steinke
    <rsteinke@w-link.net>
[reply]
[d/l]

      No, that would only be the case if the (last set of) parens weren't there.

[reply]

        Nope he is correct, it was a problem with 'and' having lower precedence than ','

        grep ( ( expression) and ( expression) and ( expression ) , @files )

        [download]

        Is interpreted as

        grep ( ( expression) and ( expression) and ( ( expression ) , @files )
+ )

        [download]

        According to perldoc -f grep, the construct grep( expression, list ) is allowed. But it is indeed confusing to man and machine!.

        Have Fun

[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom