tags:

views:

61

answers:

5

I'm using this small snippet to determine whether or not a URL is currently being stored in an array:

if( $self->{_local} eq "true" && ! grep {m|^$new_href?$|} @m_href_array ) {
    push( @m_href_array, $new_href );
    push( @href_array, $new_href );
}

It seems to work but then my code throws an error saying:

Sequence (?$...) not implemented in regex; marked by <-- HERE in m/^javascript:SearchGo(?$ <-- HERE / at C:/Perl/site/lib/ACTC.pm line 152, <> line 1.

Can anyone explain why this is happening?

+4  A: 

You don't need regexp here. Just use eq:

grep { $_ eq $new_href } @m_href_array

Also it's a good idea to use hash instead of array for faster checking:

my %allready_used_url;

if ( $self->{_local} eq "true" && ! exists $allready_used_url{ $new_href } ) {
    $allready_used_url{ $new_href } = 1; ## add url to hash
    push( @m_href_array, $new_href );
    push( @href_array, $new_href );
}
Ivan Nevostruev
A: 

What do you mean by the ? in $new_href?? Assuming there's a string in $new_href, do you expect the last letter of the string to be optional? That's not how the RE parser reads it.

zigdon
A: 

Looks like the value of $new_herf is javascript:SearchGo( which when substituted in the regex check looks like:

^javascript:SearchGo(?$

which is a broken regex as there is no matching ) for (

codaddict
+6  A: 

When searching for a string in an array, you can just use eq, rather than a regular expression:

grep { $_ eq $new_href } @m_href_array

However, if you really do need to use a regular expression (for example you are searching for a string matching a substring in the array, you should always quote the string, so that embedded special characters in your string do not have undesired effects:

grep { /\Q$substr\Esomething_else/ } @array

Moreover, if all you care about is whether the value is there, somewhere, you can short-circuit as soon as you've found a match:

use List::Util 'first';
if (first { $_ eq $new_href } @m_href_array) { ... }

or

use List::MoreUtils 'any';
if (any { $_ eq $new_href } @m_href_array) { ... }

If you're going to be doing a lot of searches, or your array is really long, you can make the process faster still by transforming the array into a hash, so you have O(1) lookups:

my %values_index;
@values_index{@array} = ();

if (exists $values_index{$element}) { ... }
Ether
A: 

You're using the URL as the pattern, and it's not a valid pattern. That's not so bad because there are much better ways to do this. The smart match makes it almost trivial:

 use 5.010;

 if( $new_href ~~ @urls ) { ... }
brian d foy