From c1046bab2e6681f5e4714178d6cc1cd16f067382 Mon Sep 17 00:00:00 2001 From: Tom Ryder Date: Sat, 16 Dec 2017 13:02:34 +1300 Subject: Move Getopt param/reqd defs into dedicated hash This shift and its comment makes what the values of the hashref passed to the validate() methods mean clearer, and also allows the use of the keys as a means of determining whether arg() was passed its definition in the array or hash format in a separate commit. --- lib/Monitoring/Plugin/Getopt.pm | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/Monitoring/Plugin/Getopt.pm b/lib/Monitoring/Plugin/Getopt.pm index 262e3c8..ca1cb61 100644 --- a/lib/Monitoring/Plugin/Getopt.pm +++ b/lib/Monitoring/Plugin/Getopt.pm @@ -382,15 +382,18 @@ sub arg my $self = shift; my %args; + # Param name to required boolean + my %params = ( + spec => 1, + help => 1, + default => 0, + required => 0, + label => 0, + ); + # Named args if ($_[0] =~ m/^(spec|help|required|default|label)$/ && scalar(@_) % 2 == 0) { - %args = validate( @_, { - spec => 1, - help => 1, - default => 0, - required => 0, - label => 0, - }); + %args = validate( @_, { %params }); } # Positional args -- cgit v1.2.3-74-g34f1 From 40d5fbea30c310004233ef4cd3e570c351c54233 Mon Sep 17 00:00:00 2001 From: Tom Ryder Date: Sat, 16 Dec 2017 13:08:44 +1300 Subject: Change arg() param type detect to key exists check Use the parameter label definitions from commit c1046ba to simplify the block of code that checks whether the first argument passed to arg() is one of the known param keys, rather than using a regular expression with alternating expression. This is more compact, may be marginally faster, and will make adding new parameters to this method more straightforward in future, avoiding the situation corrected in commit 4aa2aee. --- lib/Monitoring/Plugin/Getopt.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Monitoring/Plugin/Getopt.pm b/lib/Monitoring/Plugin/Getopt.pm index ca1cb61..feee002 100644 --- a/lib/Monitoring/Plugin/Getopt.pm +++ b/lib/Monitoring/Plugin/Getopt.pm @@ -392,7 +392,7 @@ sub arg ); # Named args - if ($_[0] =~ m/^(spec|help|required|default|label)$/ && scalar(@_) % 2 == 0) { + if (exists $params{$_[0]} && scalar(@_) % 2 == 0) { %args = validate( @_, { %params }); } -- cgit v1.2.3-74-g34f1 From c3f2a7cd7313868166ba205e9eff2b8be6a36ea4 Mon Sep 17 00:00:00 2001 From: Tom Ryder Date: Sat, 16 Dec 2017 13:25:34 +1300 Subject: Refactor positional arg parsing for Getopt::arg() Define the expected order of parameters by key in an array, and then apply hash slices to validate and assign the arguments including their required flags in one swoop, again hinging on the parameter definitions established in c1046ba. --- lib/Monitoring/Plugin/Getopt.pm | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/Monitoring/Plugin/Getopt.pm b/lib/Monitoring/Plugin/Getopt.pm index feee002..048b0ef 100644 --- a/lib/Monitoring/Plugin/Getopt.pm +++ b/lib/Monitoring/Plugin/Getopt.pm @@ -398,14 +398,8 @@ sub arg # Positional args else { - my @args = validate_pos(@_, 1, 1, 0, 0, 0); - %args = ( - spec => $args[0], - help => $args[1], - default => $args[2], - required => $args[3], - label => $args[4], - ); + my @order = qw(spec help default required label); + @args{@order} = validate_pos(@_, @params{@order}); } # Add to private args arrayref -- cgit v1.2.3-74-g34f1 From afa636bc749f76aed77380d39269bceecdd879d2 Mon Sep 17 00:00:00 2001 From: Tom Ryder Date: Sat, 16 Dec 2017 13:30:52 +1300 Subject: Remove unneeded params around "scalar" call Operator precedence allows leaving these out. --- lib/Monitoring/Plugin/Getopt.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Monitoring/Plugin/Getopt.pm b/lib/Monitoring/Plugin/Getopt.pm index 048b0ef..b5a9ab4 100644 --- a/lib/Monitoring/Plugin/Getopt.pm +++ b/lib/Monitoring/Plugin/Getopt.pm @@ -392,7 +392,7 @@ sub arg ); # Named args - if (exists $params{$_[0]} && scalar(@_) % 2 == 0) { + if (exists $params{$_[0]} && scalar @_ % 2 == 0) { %args = validate( @_, { %params }); } -- cgit v1.2.3-74-g34f1 From 6518f384cc20561ddc0df32414a714164dd2eb95 Mon Sep 17 00:00:00 2001 From: Tom Ryder Date: Sat, 16 Dec 2017 13:34:38 +1300 Subject: Pass params validation by ref not copy Since Params::Validate::validate() doesn't seem to actually mess with this specification, may as well pass a reference rather than bother copying the whole thing. --- lib/Monitoring/Plugin/Getopt.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Monitoring/Plugin/Getopt.pm b/lib/Monitoring/Plugin/Getopt.pm index b5a9ab4..65c872f 100644 --- a/lib/Monitoring/Plugin/Getopt.pm +++ b/lib/Monitoring/Plugin/Getopt.pm @@ -393,7 +393,7 @@ sub arg # Named args if (exists $params{$_[0]} && scalar @_ % 2 == 0) { - %args = validate( @_, { %params }); + %args = validate( @_, \%params ); } # Positional args -- cgit v1.2.3-74-g34f1 From da6fe80e84a20aaf405a89bd0203d9ce1abc097d Mon Sep 17 00:00:00 2001 From: Tom Ryder Date: Sat, 16 Dec 2017 14:09:16 +1300 Subject: Remove "scalar" call entirely Context with the lhs of the == operator forces this into scalar context anyway, making this call redundant. --- lib/Monitoring/Plugin/Getopt.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Monitoring/Plugin/Getopt.pm b/lib/Monitoring/Plugin/Getopt.pm index 65c872f..c8033d0 100644 --- a/lib/Monitoring/Plugin/Getopt.pm +++ b/lib/Monitoring/Plugin/Getopt.pm @@ -392,7 +392,7 @@ sub arg ); # Named args - if (exists $params{$_[0]} && scalar @_ % 2 == 0) { + if (exists $params{$_[0]} && @_ % 2 == 0) { %args = validate( @_, \%params ); } -- cgit v1.2.3-74-g34f1