CODEWITHSUNDEEP
perl
Filippo Lauria
Filippo Lauria

Reputation: 2064

check if file exists and if its file is not 0

I noticed that some scripts I'm revisioning are full of

if (-e $fullpath_err and -s $fullpath_err > 0) {
 ...

where $fullpath_err is an absolute file path. I want to lean the condition.

I thought to create a sub like this:

sub fileIsValid {
  my $filepath = shift;
  return (-e $filepath and -s $filepath > 0);
}

and use it this way:

if (fileIsValid($fullpath_err)) {
 ...

Is there another (maybe simpler) way to do this?

Upvotes: 0

Views: 114

Answers (3)

runrig
runrig

Reputation: 6524

If the file does not exist, then it has no size. All that is really needed is:

if ( -s $fullpath_err ) { ...

Upvotes: -1

Dave Cross
Dave Cross

Reputation: 69244

One small optimisation. The -e and -s calls will both make a stat call to the underlying operating system. This can add a small overhead to your code. Each stat call will actually return all of the data about the file, so calling it twice is wasteful. But Perl caches the data from the more recent stat against the special filehandle _, so if you use that instead of the filename on your second call, you'll save one call to stat.

sub fileIsValid {
  my $filepath = shift;
  return (-e $filepath and -s _ > 0);
}

Note: I've also removed the unnecessary and potentially problematic prototype.

Upvotes: 1

thb
thb

Reputation: 14454

Is there another (maybe simpler) way to do this?

Not really, as far as I know. Your code is good, idomatic Perl. You can prototype fileIsValid() if you like, so that it will specifically take a single, scalar argument:

sub fileIsValid ($) {
  my $filepath = shift;
  return (-e $filepath and -s $filepath > 0);
}

Even the prototype is probably unnecessary, though.

Your code is good.

Upvotes: 1

Related Questions