Any reason you couldn't do this?

sub trash
{
  my ($o, $item) = @_;
  eval ( ( -d $item ) ? rmtree ( $item ) : unlink ( $item ) );
  $o->log ( 'error', "$item": $@" ) if ( $@ );
}
[download]

Any reason you couldn't do this?

good suggestion! thanks

Carter's compass: I know I'm on the right track when by deleting something, I'm adding functionality

sub do_or_log_error {
  my $coderef = shift;
  my $syslogger = shift;
  my $param = shift;
  eval { $coderef->($param) };
  if ($@) {
    $syslogger->LOG('error', "$param: $@");
  }
}

sub my_rmtree { # yours was infinite loop!
  do_or_log_error(\&rmtree, @_);
}

sub my_unlink { # yours was infinite loop!
  do_or_log_error(sub { unlink @_ }, @_);
}
[download]

-- Randal L. Schwartz, Perl hacker
Be sure to read my standard disclaimer if this is a reply.

Refactoring is less about making the code smaller than it is making the code more supportable. Usually the two go together, but not always. If you misplace these goals, you end up with smaller, less supportable code.

Both of your routines are succinct. The only refactoring I see that doesn't significantly reduce readability would be to add something like

  sub LOGifError {
    my ($o, $what, $err) = @_;
    $o->LOG('error', "$what: $error") if $error;
  }
[download]

Or, if you want to "remove" two duplicate lines from each routine without adding a new method, rewrite

  if ($@) {
     $o->LOG('error', "$tree: $@");
  }
[download]

How would you recode these two routines for succinctness?

Maximizing succinctness is not always a good idea. Sure, it'll make you feel clever if you can reduce your program to the minimum number of lines. But it has a tendency to become a maintenance problem. What happens if you discover a slight difference in the way the parameters, return values, or errors are handled in the two functions? You either undo the succinctification, or you create a godawful mess of special cases. If your code is being maintained by someone else, the chance that they pick option two is far too high.

Unless there's something you aren't telling us (like each function is actually 100 lines long, or there are 100 of these functions), it seems like you'd be better off applying your hubris somewhere it'll get better returns.

What is "rmtree"?

I can hazard a guess as to what it does, but is it a OS command, a function from a module?

Also, why are you evaling built-ins?

In my (breif) tests, nothing ever gets put into $@ whether or not the function succeeded or failed. Any error is always available in $! and never $@.

I'm not at all sure of the utility of putting built-ins into your own subs? What's wrong with

unlink $file or $o->LOG( 'error', "$file: $!");
rmdir $tree or $o->LOG( 'error', "$tree: $!");
[download]

Update:Do all the other responders know something I don't? They all go along with testing and printing $@???

What is "rmtree"? I can hazard a guess as to what it does, but is it a OS command, a function from a module?

rmtree is a sub in File::Path

Carter's compass: I know I'm on the right track when by deleting something, I'm adding functionality

Okay, that makes sense. However, I just tried to create errors by deleting a) a non-existant subdir. b) a subdir which I did not have permission to delete. I tried this using eval as shown in your post and again in neither case did anything end up in $@. I did get various messages to STDERR:

Can't make directory testtesttest read+writeable: Permission denied at
+ C:\test\QOTW\test.pl line 27
Can't read testtesttest: Invalid argument at C:\test\QOTW\test.pl line
+ 27
Can't make directory testtesttest writeable: Permission denied at C:\t
+est\QOTW\test.pl line 27
Can't remove directory testtesttest: Permission denied at C:\test\QOTW
+\test.pl line 27
and can't restore permissions to 0777
Should be no errors
$!: 'Permission denied'
$@: ''
Can't make directory testtesttest read+writeable: Permission denied at
+ C:\test\QOTW\test.pl line 31
Can't read testtesttest: Invalid argument at C:\test\QOTW\test.pl line
+ 31
Can't make directory testtesttest writeable: Permission denied at C:\t
+est\QOTW\test.pl line 31
Can't remove directory testtesttest: Permission denied at C:\test\QOTW
+\test.pl line 31
and can't restore permissions to 0777
Should be errors
$!: 'Permission denied'
$@: ''
[download]

Looking at the docs I saw this:

Note also that the occurrence of errors in rmtree can be determined only by trapping diagnostic messages using $SIG{__WARN__}; it is not apparent from the return value.

---

Okay you lot, get your wings on the left, halos on the right. It's one size fits all, and "No!", you can't have a different color.
Pick up your cloud down the end and "Yes" if you get allocated a grey one they are a bit damp under foot, but someone has to get them.
Get used to the wings fast cos its an 8 hour day...unless the Govenor calls for a cyclone or hurricane, in which case 16 hour shifts are mandatory.
Just be grateful that you arrived just as the tornado season finished. Them buggers are real work.

from Programming Perl:
$@
The currently raised exception or the Perl syntax error message from the last eval operation. (Mnemonic: where was the syntax error "at"?) Unlike $! ($OS_ERROR), which is set on failure but not cleared on success, $@ is guaranteed to be set (to a true value) if the last eval had a compilation error or run-time exception, and guaranteed to be cleared (to a false value) if no such problem occurred. (...)

HTH

That's all very well, but if the file/dir string passed to unlink/rmtree simply doesn't exist or script does not have sufficient rights to perform the deletion, the will be neither a compilation error nor a run-time exception raised. I know cos I tried both situations. Therefore $@ will never be set, so neither failure will be detected.

$! is set in both cases.

I therefore can see no benefit in evaling these calls to functions. Better to use the standard idiom of unlink $file or die/warn/LOG $!; and rmtree $dir or die/warn/LOG $!; don't you think?

---

Okay you lot, get your wings on the left, halos on the right. It's one size fits all, and "No!", you can't have a different color.
Pick up your cloud down the end and "Yes" if you get allocated a grey one they are a bit damp under foot, but someone has to get them.
Get used to the wings fast cos its an 8 hour day...unless the Govenor calls for a cyclone or hurricane, in which case 16 hour shifts are mandatory.
Just be grateful that you arrived just as the tornado season finished. Them buggers are real work.

## untested
sub AUTOLOAD {
    my $sub = $AUTOLOAD;
    croak("Unknown method: $sub")
      unless defined &$sub;

    my($o, $arg) = @_;
    eval { &$sub( $arg ) };
    $o->LOG("error", "$arg: $@") if $@;    
}
[download]

_________
broquaint

sub rmtree {
   trashIt(@_, "rmtree");
}

sub unlink {
  trashIt(@_, "unlink");
}

sub trashIt {
  my ($o, $file, $cmd) = @_;

  eval { $cmd $file } ;

  if ($@) {
    $o->LOG('error', "$file: $@");
  }
}
[download]