Skip to content

Values::ArgumentError - exception class containing missing/unexpected constructor arguments #51

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tomeon
Copy link

@tomeon tomeon commented Dec 8, 2015

I hesitate to make this PR given that part of Values' appeal is its exceptionally small size; so, apologies in advance if the changeset it unwanted, and thanks for taking the time to review it.

I use Values for input validation and would find it helpful if the exceptions raised by the .new and .with methods upon receipt of bad arguments contained lists of missing and unrecognized keys, along the same lines as how virtus's CoercionError exception class has the .attribute_name method for helping to determine which failed attribute prevented an object's instantiation.

Since Values already validates constructor arguments, this PR mostly amounts to percolating existing data up to the caller -- the exception being the second argument to the Values::ArgumentError created in .new.

The convention I've followed is that the @missing_keys and @unexpected_keys attributes will contain empty arrays if there were, respectively, no missing arguments or no extra arguments to .with. By contrast, @unexpected_keys will be nil if there were extra arguments provided to .new, since there is no way to determine the 'meaning' of additional positional parameters in the absence of handy hash keys acting a labels.

@tcrayford
Copy link
Owner

@BaxterStockman whilst I agree on Values' small size, I like this PR and would like to merge it. I have some nitpicks that I'd like addressed first.

@@ -10,6 +10,17 @@
# p.y
# #=> 0
#
module Values
class ArgumentError < ::ArgumentError
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please to rename to FieldError

1. {missing,unexpected}_keys => {missing,unexpected}_values
2. Values::ArgumentError => Values::FieldError
3. Default values for @{missing,unexpected}_fields now [] instead of nil
@tomeon
Copy link
Author

tomeon commented Dec 9, 2015

@tcrayford -- many thanks for your feedback! I've updated the PR in response to your comments.

@ms-ati
Copy link
Contributor

ms-ati commented Dec 28, 2015

@tcrayford -- are you planning to merge this and release a 1.10.0?

@ms-ati
Copy link
Contributor

ms-ati commented Apr 5, 2016

ping

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.

3 participants