Re: Re: Copy Permissions

Regarding: "The next thing that bothers me is in your $wanted->() sub. You're not checking for the validity of your pattern match, and then you're subsequently relying on $2 to contain a reasonable value. Don't rely on captures unless you're certain that there was a match. Just don't; you will get into trouble eventually with that approach."

How would you go about ensuring the match? Would you just do a if to test if there is something there, or would there be a remenant from the last match? If there was a remenant would you need to assign it to a global and then first test if it is the same as the global?

Any suggestions would be greatly welcomed.

-Shane

Comment on Re: Re: Copy Permissions

Replies are listed 'Best First'.

Re: Re: Re: Copy Permissions
by davido (Cardinal) on Apr 26, 2004 at 23:01 UTC

my $wanted = sub { $name =~ /\Q($wrong_dir\E)(.*)/; my $the_thing = "$correct_dir$2"; if (-e $the_thing) { cpmod2($the_thing,$name) or die "just couldnt do it: $!\n"; } };
[download]

How would you go about ensuring the match?

The point is that $2 can contain an artifact of previous match successes, it can contain undef, or under some circumstances, it can contain rubbish. Your script is set up to die if it's unable to perform the desired action in the $wanted->() sub. Error handling of the match could occur one of several ways, depending on what you think is best.

You can either issue a warning and then break out of the sub before taking the next action, or die altogether. I kind of like the idea of issuing a warning and breaking out of the sub so that the next iteration can still take place.

my $wanted = sub
{
    $name =~ /\Q($wrong_dir\E)(.*)/ or do {
        warn "Can't match $name for $wrong_dir in line __LINE__.\n";
        return undef;
    };
    my $the_thing = "$correct_dir$2";
    if (-e $the_thing)
    {
        cpmod2($the_thing,$name) or die "just couldnt do it: $!\n";
    }
};
[download]

That should at least guarantee that you're never attempting to do a cpmod2() where $name is potentially unknown garbage.

Of course there's always more than one way to do it.

One more point I wanted to make clear: My critique was in no way intended to discourage you or anyone else from posting code here, whether it's pristine or somewhat less than that. I am glad when I see code posted, and my critique was only intended to provide a basis from which you and others can pick up on areas for improvement that will help in the longrun. I did notice your discussion in the CB regarding the posting of code for critique, so I assumed that was what you had in mind with this particular piece. Overall, good job. I don't think it's possible to post anything to PerlMonks without someone chiming in with better ways (in his/her opinion) to do things.

Take my critique as one person's opinion. If some of what I've said is found to be beneficial to you, that's all I can hope for. Keep up the good work.

Dave

[reply]
[d/l]
[select]