Re: [PATCH v2 15/22] git-remote-mediawiki: Put long code into a subroutine
From: Eric Sunshine <hidden>
Date: 2016-06-15 22:57:36
On Fri, Jun 7, 2013 at 5:42 PM, Célestin Matte [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Signed-off-by: Célestin Matte <redacted> Signed-off-by: Matthieu Moy <redacted> --- contrib/mw-to-git/git-remote-mediawiki.perl | 42 +++++++++++++++++---------- 1 file changed, 26 insertions(+), 16 deletions(-)diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl index 1c34ada..f37488b 100755 --- a/contrib/mw-to-git/git-remote-mediawiki.perl +++ b/contrib/mw-to-git/git-remote-mediawiki.perl@@ -133,22 +133,7 @@ while (<STDIN>) { @cmd = split(/ /); if (defined($cmd[0])) { # Line not blank - if ($cmd[0] eq "capabilities") { - die("Too many arguments for capabilities\n") if (defined($cmd[1])); - mw_capabilities(); - } elsif ($cmd[0] eq "list") { - die("Too many arguments for list\n") if (defined($cmd[2])); - mw_list($cmd[1]); - } elsif ($cmd[0] eq "import") { - die("Invalid arguments for import\n") if ($cmd[1] eq "" || defined($cmd[2])); - mw_import($cmd[1]); - } elsif ($cmd[0] eq "option") { - die("Too many arguments for option\n") if ($cmd[1] eq "" || $cmd[2] eq "" || defined($cmd[3])); - mw_option($cmd[1],$cmd[2]); - } elsif ($cmd[0] eq "push") { - mw_push($cmd[1]); - } else { - print STDERR "Unknown command. Aborting...\n"; + if (parse_commands() == 1) { last;
Better. A few minor nits:
The new subroutine is only parsing/invoking a single command at a time
from the input line rather than multiple commands, so the name
parse_commands() is slightly misleading. Perhaps parse_command() would
be more appropriate.
Now that the functionality has been pushed into a subroutine, it does
not necessarily need to be accessing global @cmd. It might be
appropriate to pass @cmd as an argument to parse_command() (or not
depending upon your preference).
The "Unknown command. Aborting" case indicates a failure. It's pretty
typical for failures to return false rather than true. The resulting
conditional would then read a bit more idiomatically:
if (!parse_command(\@cmd)) {
last;
}
quoted hunk ↗ jump to hunk
} } else {@@ -168,6 +153,31 @@ sub exit_error_usage { die "ERROR: git-remote-mediawiki module was not called with a correct number of parameters\n"; } +sub parse_commands { + if ($cmd[0] eq "capabilities") { + die("Too many arguments for capabilities\n") + if (defined($cmd[1])); + mw_capabilities(); + } elsif ($cmd[0] eq "list") { + die("Too many arguments for list\n") if (defined($cmd[2])); + mw_list($cmd[1]); + } elsif ($cmd[0] eq "import") { + die("Invalid arguments for import\n") + if ($cmd[1] eq "" || defined($cmd[2])); + mw_import($cmd[1]); + } elsif ($cmd[0] eq "option") { + die("Too many arguments for option\n") + if ($cmd[1] eq "" || $cmd[2] eq "" || defined($cmd[3])); + mw_option($cmd[1],$cmd[2]); + } elsif ($cmd[0] eq "push") { + mw_push($cmd[1]); + } else { + print STDERR "Unknown command. Aborting...\n"; + return 1; + } + return 0; +} + # MediaWiki API instance, created lazily. my $mediawiki; --