Here are some comments on your code
I hope the call to
commit_to_perforce
is in a different file from the method definition?It is often much clearer to define a hash parameter separately and then pass a reference to it, rather than passing an anonymous hash directly in the parameter list
Within a method, it is usually best to
shift
the vlaue of$self
off the parameter list, and then do a list assignment for the rest of the parametersYou should use the three-parameter form of
open
, and either check that it succeeded with adie
string that includes$!
to give the reason for the failure, or justuse autodie
.Because of the precedence of the
||
operator, your code checks the truth of the string">$local_metadata_file"
instead of the return value fromopen
. You can either use the low-precedenceor
operator instead or put parentheses around the parameters toopen
It is common practice to reserve upper case letters for global identifiers, such as the package
Dumper
. Local variables should normally be named using lower-case letters, numbers, and underscore
Taking all of that into account, here's how your code should look
my %meta = (
size_of_table => $row->{numRecords},
);
$self->commit_to_perforce($network_name[0], \%meta);
sub commit_to_perforce {
my $self = shift;
my ($network, $metadata) = @_;
open my $fh, '>', $local_metadata_file
or die "Could not open file '$local_metadata_file' for output: $!";
print $fh Dumper $metadata;
close $fh;
}
I hope this helps