Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up argument adding code a little bit #17

Merged
merged 6 commits into from Dec 22, 2017
Merged

Clean up argument adding code a little bit #17

merged 6 commits into from Dec 22, 2017

Conversation

tejr
Copy link

@tejr tejr commented Dec 16, 2017

This changeset streamlines the code in the subroutine Monitoring::Plugin::Getopt::arg() that is used for validating and adding new arguments to the plugin, completing some changes I've wanted since my commit 4aa2aee to make the code a bit more robust if and when new argument parameters are introduced. I have run the included test suite on Perl 5.26.1 and Perl 5.8.1 and it passes.

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.
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.
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.
Operator precedence allows leaving these out.
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.
Context with the lhs of the == operator forces this into scalar context
anyway, making this call redundant.
@sni sni merged commit 00660df into monitoring-plugins:master Dec 22, 2017
@sni
Copy link
Contributor

sni commented Dec 22, 2017

Thank you very much, looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants