[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[patch] configNow: 3 arg open, no return undef


fix(ConfigNow.pm): 3 argument open now used

Three argument open is preferred over two argument open. It keeps
the file and the file input mode separated, which is important because
it removes one possible area for bugs to hide.

fix(Module.pm): return undef is now a bare return

The problem with returning undef to signify failure in a subroutine
is this: if that subroutine is called in list context, it'll return
(undef). Which evaluates to true in a boolean context since it's a
list with at least one element. It's better to use a bare return
statement when you want to return failure.
diff --git a/lib/IRCNOW/ConfigNow.pm b/lib/IRCNOW/ConfigNow.pm
index 44aa8bc..3ef6584 100644
--- a/lib/IRCNOW/ConfigNow.pm
+++ b/lib/IRCNOW/ConfigNow.pm
@@ -107,7 +107,7 @@ sub write_file {
 	$filename = "$workDir/$filename";
 	my $path = dirname($filename);
 	make_path($path);
-	open my $FH, ">>$filename" || die "failed to open $filename for appending";
+	open my $FH, ">>", $filename || die "failed to open $filename for appending";
 	print $FH $output;
 	close $FH;
 }
@@ -118,7 +118,7 @@ sub read_file {
 	my $workDir = shift || $self->{vars}->{gitWorkDir} || die 'No work directory specified';
 	$filename = "$workDir/$filename";
 	my $output="";
-	open (my $FH, "<$filename") || die "failed to open $filename for reading";
+	open (my $FH, "<", $filename) || die "failed to open $filename for reading";
 	while (<$FH>) {
 		$output.= $_;
 	}
@@ -328,7 +328,7 @@ sub template_head {
 	my $output = "";
 	if (-e "$templates/$filename" . "_HEAD") {
 print "Reading: $templates/$filename" . "_HEAD\n";
-		open (my $FH, "<$templates/$filename" . "_HEAD");
+		open (my $FH, "<", "$templates/$filename" . "_HEAD");
 		while (<$FH>) {
 			$output.=$_;
 		}
diff --git a/lib/IRCNOW/ConfigNow/Module.pm b/lib/IRCNOW/ConfigNow/Module.pm
index ea451bb..33636fb 100644
--- a/lib/IRCNOW/ConfigNow/Module.pm
+++ b/lib/IRCNOW/ConfigNow/Module.pm
@@ -109,7 +109,7 @@ sub output {
 		for my $param ( @{$mod->{varlist}}) {
 			# Don't have needed param so return undef
 			unless (exists $self->{vars}->{$param}) {
-				return undef;
+				return;
 			}
 			push @params, $self->{vars}->{$param};
 		}
@@ -118,7 +118,7 @@ sub output {
 		}
 		return sprintf($mod->{template}, @params);
 	}
-	return undef; #nothing to output 
+	return; #nothing to output
 }