Better way to do this, regex etc.

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

Hi. I'm a complete perl novice. I've knocked together some code to do what I want but I know there must be a more efficient way to do this. Can anyone improve on this code? Thanks in advance. Mint

#!/usr/bin/perl -w

use strict;
use warnings;


# I want to parse a string containing bug fix identifiers such that
# it will take a list of comma separated bugs, or single bugs prefixed
# with "bug[s:]" ignoring whitespace, sort them and pick out only
# unique identifiers and put them into an array.
# The printing is just for testing each stage.

my $bug_fix_msg =       "this 33,44 is a fix for mybug44,99 real \
                        bug 99 again   bug33r bug1722,3 bUg:99, \
                        bugs:456, 17,   24 bugs 42,58 bug:  50,50";
my @filtered  = ();

while ($bug_fix_msg =~ m/\bbugs?:?([\s\d,]*)\b/ig) {
        push @filtered, $1;
        print $1;
        print "\n";
}
print "filtered: @filtered";

s/\s//g for(@filtered);

my @bug_array = ();

foreach my $bug_line(@filtered) {
        my @items = split(',', $bug_line);
        push @bug_array, @items;
}

my %hash   = map { $_ => 1 } @bug_array;
my @unique = keys %hash;
my @sorted = sort {$a <=> $b} @unique;

print "\nunsorted: @bug_array";
print "\n  unique: @unique";
print "\n  sorted: @sorted\n";
[download]

The output looks like this -

 99
1722,3
99,

456, 17,   24
 42,58
  50,50
filtered:  99  1722,3  99,
                         456, 17,   24   42,58    50,50
unsorted: 99 1722 3 99 456 17 24 42 58 50 50
  unique: 50 3 58 17 456 42 99 24 1722
  sorted: 3 17 24 42 50 58 99 456 1722
[download]

Comment on Better way to do this, regex etc. Select or Download Code

Replies are listed 'Best First'.
Re: Better way to do this, regex etc. by Tanktalus (Canon) on Mar 03, 2009 at 23:53 UTC
Here's my rewrite: `#!/usr/bin/perl -w use strict; use warnings; use Sort::Key qw(usort); use List::MoreUtils qw(uniq); my $bug_fix_msg = "this 33,44 is a fix for mybug44,99 real \ bug 99 again bug33r bug1722,3 bUg:99, \ bugs:456, 17, 24 bugs 42,58 bug: 50,50"; my @bug_array = map { split /\s,\s/, $_ } $bug_fix_msg =~ m/\bbugs?\s:?\s(\d+(?:\s,\s\d+))\b/ig; my @sorted = usort uniq @bug_array; print "\n sorted: @sorted\n"; print "OK!\n" if "@sorted" eq "3 17 24 42 50 58 99 456 1722";` [download] I started with your same test, and ended by ensuring that the sorted output matched what you had before. So, it at least passes this test. Just some quick notes... I drastically changed your regex. Instead of matching multiple commas between numbers, I now only match digits-(spaces?-one_comma-spaces?-digits). So, "123, 456" is good, "123,,456" is discarded. Then I split each of the regex matches, and the split is that space-comma-space section. This gets rid of the spaces at the same time as doing the split, which makes it a bit less work to read/understand/execute. Then the unique/sorting steps, I went to modules. List::MoreUtils is one of my favourite modules for useful tidbits of code where I don't have to worry about bugs. It's often faster than a hand-written version due to being XS, but it's always faster in developer time, once you're familiar with them. I don't have to think about it - I just call "uniq LIST". And then, rather than doing $a vs $b comparisons, I know that Sort::Key, which is also mostly native XS code, already does this. This shouldn't be much different than your version, but can be a tiny bit more explicit. Sort::Key has many other options for sorting, too, so you may want to get familiar with it anyway. Hope that helps. Update: Thank you for a) posting your code, b) describing what you wanted to do, and c) providing a built-in test that others could use to get a better idea of what you're talking about. Oh, and d) providing a negative test ("bug33r"). All VERY helpful. I wish I could give a ++ for each of these points.	[reply] [d/l]
Re: Better way to do this, regex etc. by BrowserUk (Patriarch) on Mar 04, 2009 at 01:32 UTC
Better? Probably not. Just for fun! `#! perl -slw use strict; my $bug_fix_msg = "this 33,44 is a fix for mybug44,99 real \ bug 99 again bug33r bug1722,3 bUg:99, \ bugs:456, 17, 24 bugs 42,58 bug: 50,50"; my @bugs = sort{$a<=>$b} keys %{{ map { map{ $_ => 1 } split ',\s', $_; } $bug_fix_msg =~ m[ \bbug s? :? \s ( \d+\b (?: , \s* \d+\b )* ) ]gmsx }}; print "@bugs"; __END__ C:\test>junk3 3 17 24 42 50 58 99 456 1722` [download] Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error. "Science is about questioning the status quo. Questioning authority". In the absence of evidence, opinion is indistinguishable from prejudice. "Too many [] have been sedated by an oppressive environment of political correctness and risk aversion."	[reply] [d/l]
Re: Better way to do this, regex etc. by jwkrahn (Abbot) on Mar 04, 2009 at 01:10 UTC
Here is another way to do it: `#!/usr/bin/perl use warnings; use strict; my $bug_fix_msg = "this 33,44 is a fix for mybug44,99 real \ bug 99 again bug33r bug1722,3 bUg:99, \ bugs:456, 17, 24 bugs 42,58 bug: 50,50"; my %unique; while ( $bug_fix_msg =~ /\bbugs?:?([\s\d,]*)\b/ig ) { print "$1\n"; $unique{ $_ }++ for $1 =~ /\d+/g; } my @sorted = sort { $a <=> $b } keys %unique; print "unique: @{[keys %unique]}\n"; print "sorted: @sorted\n";` [download]	[reply] [d/l]
Re^2: Better way to do this, regex etc. by mintz (Initiate) on Mar 04, 2009 at 10:48 UTC
Hey thanks to everyone for your help. I decided to use jwkrahn's method, but I also realised that my regex wouldn't reject "bug,,8 bug,9". So I've added a lookahead to the regex filter. As follows: #!/usr/bin/perl use warnings; use strict; my $bug_fix_msg = "this 33,44 is a fix for mybug44,99 real \ bug 99 again bug33r bug1722,3 bUg:99, \ bugs:456, 17, 24 bugs 42,58 bug: 50,50 \ buggy99 bug3,:8,34 bug,,49 bug,84 bug:s12"; print "test string: $bug_fix_msg\n\nFiltered:\n"; my %unique; while ( $bug_fix_msg =~ /\bbugs?:?(?!,)([\s\d,]*)\b/ig ) { print "$1\n"; $unique{ $_ }++ for $1 =~ /\d+/g; } print ("End Filtered\n\n"); my @sorted = sort { $a <=> $b } keys %unique; print " unique: @{[keys %unique]}\n"; print " sorted: @sorted\n"; [download] Output as follows: `test string: this 33,44 is a fix for mybug44,99 real bug 99 again bug33r bug1722,3 bUg:99, bugs:456, 17, 24 bugs 42,58 bug: 50,50 buggy99 bug3,:8,34 bug,,49 bug,84 bug:s12 Filtered: 99 1722,3 99, 456, 17, 24 42,58 50,50 3 End Filtered unique: 50 3 58 17 456 42 99 24 1722 sorted: 3 17 24 42 50 58 99 456 1722` [download] Thanks again for the help. I've always thought of regex parsing as a bit of a black art, but I'm starting to get the hang of it.	[reply] [d/l] [select]