Tek-Tips is the largest IT community on the Internet today!

Members share and learn making Tek-Tips Forums the best source of peer-reviewed technical information on the Internet!

  • Congratulations SkipVought on being selected by the Tek-Tips community for having the most helpful posts in the forums last week. Way to Go!

Grep testing expression within subroutine

Status
Not open for further replies.

Zhris

Programmer
Aug 5, 2008
254
0
0
GB
I am trying to combine grep with a basic subroutine, however it does not work correctly. The output list is identical to the input list...

Code:
my @input = (1,2,4,8,16,32,64,128);
my @output = grep square($_),@input;
sub square{
	my $value = shift;
	my $result = $value * $value;
	return $result;
}
print "All values squared (Subroutine) = @output<p>";
#All values squared (Subroutine) = 1 2 4 8 16 32 64 128

However, this works...

Code:
my @input = (1,2,4,8,16,32,64,128);
my @output = grep square($_),@input;
sub square{
	$_ = $_ * $_;
	return $_;
}
print "All values squared (Subroutine) = @output<p>";
#All values squared (Subroutine) = 1 4 16 64 256 1024 4096 16384

What am I doing wrong in the first snippet of code?

Thank you.
 
Hm. That's pretty weird. I'm not sure what's going on there, but I can tell you that this isn't how grep is supposed to be used -- it's used to find stuff in a list, but not operate on it. I think what you want is map. Check out these examples. Sorry I can't tell you about the weird results.

Code:
#!/usr/bin/perl
use strict;

my @input = (1,2,4,8,16,32,64,128);
my @output = grep(/\d{2}/, @input);
my @output2 = map($_ * $_,@input);

print "grep = @output\n";
print "map = @output2\n";

#output
#grep = 16 32 64 128
#map = 1 4 16 64 256 1024 4096 16384

--
 
Okay, I see what's going on. grep returns the elements of the list when the expression returns true. Your 2nd snippet actually modifies @input, evaluates to true, and returns that element. The 1st snippet just does some irrelevant calculations, evaluates to true, and returns the element from @input. Check this out.

Code:
#!/usr/bin/perl
use strict;

my @input = (1,2,4,8,16,32,64,128);
my @output = grep square($_),@input;
print "input = @input\n";
print "output = @output\n\n";

my @output2 = grep square2($_),@input;
print "input = @input\n";
print "output2 = @output2\n\n";

my @output3 = grep square3($_),@input;
print "input = @input\n";
print "output3 = @output3\n\n";

sub square{
    my $value = shift;
    my $result = $value * $value;
    return $result;
}

sub square2{
    $_ = $_ * $_;
    return $_;
}

sub square3{
    $_ = $_ * $_;
    return 0; # or return false, which means it didn't match
}

#prints:
#input = 1 2 4 8 16 32 64 128
#output = 1 2 4 8 16 32 64 128

#input = 1 4 16 64 256 1024 4096 16384
#output2 = 1 4 16 64 256 1024 4096 16384

#input = 1 16 256 4096 65536 1048576 16777216 268435456
#output3 =

--
 
There is no need for grep at all:

Code:
my @input = (1,2,4,8,16,32,64,128);
my @output = square(@input);
sub square{
    my @value = @_;
    $_ *= $_ for @value;
    return @value;
}
print "All values squared (Subroutine) = @output";

The only reason you would use grep in that context is to see if a value returns true or false from the subroutine.

------------------------------------------
- Kevin, perl coder unexceptional! [wiggle]
 
I am currently reading O'Reilly's Intermediate Perl book and thought I may as well create my own examples as I go along. A very similar example to what I tried is provided, and I understand that my method is inefficient (map is better suited) or I could have used...

Code:
my @output = grep{$_ =  $_ * $_;}@input;

Thank you for explaining why my code doesn't work, and that my example is pretty irrelevent :).

 
That is also a bad example because $_ is a reference back to the original element of the list:

Code:
my @input = (1,2,4,8,16,32,64,128);
grep{$_ =  $_ * $_;}@input;
print "@input";

Notice @input has changed. grep is for testing a condition, not applying an expression.

Quote from grep manpage:

Note that, because $_ is a reference into the list value, it can be used to modify the elements of the array. While this is useful and supported, it can cause bizarre results if the LIST is not a named array. Similarly, grep returns aliases into the original list, much as a for loop's index variable aliases the list elements. That is, modifying an element of a list returned by grep (for example, in a foreach, map() or another grep() ) actually modifies the element in the original list. This is usually something to be avoided when writing clear code.

------------------------------------------
- Kevin, perl coder unexceptional! [wiggle]
 
Code:
my @output = grep{$_ =  $_ * $_;}@input;
I would avoid doing it this way as much as possible, for a number of reasons. Firstly, you're modifying @input (if you change $_ in a 'grep' expression, you alter the relevant value in the original array and then you're making a duplicate of that altered array in @output. That seems pretty counter-intuitive to me. If you want to modify @input, then do this:
Code:
$_ = $_*$_ for @input;
It's also counter-intuitive because you're not using 'grep' for what it's intended to be for, as Kevin has noted. The 'grep' function is for identifying elements in a list that satisfy a particular condition, and returning those (omitting those that do not satisfy the condition). You're essentially relying on the side-effects of 'grep' to do what you want. That's a dangerous game though, especially if you don't totally understand the function you're using. A subtle bug in your code arises when you have the value '0' in your array. Squaring 0 gives you 0, which means that grep will not include that value in your @output array.

What you're looking for is to use 'map':
Code:
my @output = map $_*$_, @input;
That doesn't alter @input while ensuring that every element in @input gets squared and the result added to @output.
 
Status
Not open for further replies.

Part and Inventory Search

Sponsor

Back
Top