Re: Generic Parsing class with callback

I would force the caller to send in a callback instead of doing eval on a string...

use warnings;
use strict;

sub work {
    my ($x, $y, $callback) = @_;

    if (ref $callback ne 'CODE'){
        die "callback param needs to be a code reference";
    }

    $callback->($x, $y);
}

# with a ref to an existing sub

sub this {
    my ($x, $y) = @_;
    print "$x $y\n";
}

work(3, 4, \&this);

# with an explicit code ref

my $cref = sub { print "$_\n" for @_; };
work(2, 2, $cref);

# or inline

work(2, 3, sub { print "$_[0], $_[1]\n"; })
[download]

...and in your documentation, clearly state what parameters (the number and type) the callback should expect to receive.

It doesn't look like you need an eval later on either. eval is pretty much a try/catch to ensure no problems happened, and puts any errors it encounters in $@ so you can handle them gracefully. I think your else could be re-written as such (untested), given that it looks like you're only trying to see if a match was made, not the matches themselves:

my $regex = qr/$self->{regex}/;
while(<$input>){
    my $result = /$regex/;
        $self->{callback_ref}($result,$line,$output);
}
[download]

-stevieb

Comment on Re: Generic Parsing class with callback Select or Download Code

Replies are listed 'Best First'.
Re^2: Generic Parsing class with callback by QuillMeantTen (Friar) on Aug 31, 2015 at 20:50 UTC
I tried storing the regex with a simple qr but it broke whenever I tried to do a substitution regex, that's why I chose this quite unelegant way to handle it. If you have a better idea though I'd gladly change the way its written :) everything else was very good advice, it looks better but most importantly it feels cleaner. If you'd be so kind, have a look at the improved code! Update : the use of the eval might not seem obvious, this submodule has been written to help normalize Logs before putting them into a database hence the provisioning for substitutions, I want to extract metadata to fill dublin core style tables Since I plan to use the same objects to scrap the database for suspicious behaviour pattern matching also was of interest Read more... (3 kB) and the updated test file Read more... (3 kB)	[reply] [d/l] [select]
Re^3: Generic Parsing class with callback by stevieb (Canon) on Aug 31, 2015 at 21:33 UTC
I'll take a closer look later when I have more time, but I didn't realize you were doing substitution regexes. For that, you either have to use `eval` or here are two other methods that I know how to get around the `eval` call. Force the caller to pass in a code reference with the entire details of the substitution... `my $str = "This is one thing. This."; sub cref_regex { my ($str, $re_cref) = @_; print $re_cref->($str); } my $cref = sub { $_[0] =~ s/This/That/ig; }; cref_regex($str, $cref);` [download] ...or, force the caller to send in both a `$search` and if required, an optional `$replace` variable... `sub separate_regex { my ($str, $search, $replace) = @_; if (! $replace){ print $str =~ /$search/; } else { print $str =~ s/$search/$replace/g; } } separate_regex($str, 'This', 'That');` [download] Both of those are more work for the caller, so I can't tell which is better in the long run. I've just always heard "don't use string `eval`", so I try to avoid it where possible. In a recent module of mine I updated, I opted for the `$search, $replace` option. -stevieb	[reply] [d/l] [select]