-
-
Notifications
You must be signed in to change notification settings - Fork 242
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
Replaced #add_indifferent_access with #stringify #142
Replaced #add_indifferent_access with #stringify #142
Conversation
Actually, this does change the behaviour of defaults slightly. Whereas before if you insert defaults, these get added into your existing data structure, now you'll also find all symbols converted to strings in your data structure (only if you insert defaults) |
This might also fix #95 |
I think there is a little smell when a data validation library changes the existing parts of the data it receives. |
Yeah, I agree - I'd rather if validation didn't change the data structure it was using. How about we deprecate insert_defaults, but instead add a new method which doesn't validate, but just inserts the defaults and returns a new object with the defaults inserted (but nothing else)? Or (more controversial) just remove insert_defaults. They would still be used for validation. |
I would not abandon insert defaults, I think its a useful feature. |
No, I think that right now it inserts into the original as it goes through |
We shouldn't break the behavior of insert_defaults without a major version tick; now that @hoxworth has added a v3 milestone, maybe this belongs there, unless there's a way to get the code simplification without breaking backwards compatibility. 👍 in general, though; if there's any discussion of a v3 roadmap, some form of "drop support for symbols" is gonna be high up on my list of suggestions. =) |
I agree it's a noticeable api change. |
baa4cb6
to
c731fe3
Compare
Right guys, I've fixed this up now so that it preserves the old behaviour. If |
This is a fix for voxpupuli#96 Previously, add_indifferent_access would modify the data and schema hashes in place, to make them accept strings or hashes as keys. However, there are two problems with this: 1. It works for accessing the hash (eg. fetch or []) but it does not work if we use .has_key? or .keys 2. It does not convert arrays of symbols into arrays of strings (which can be a problem when the symbols in the array refer to attributes, eg. required) To resolve this, I've removed add_indifferent_access in favour of stringify, which returns a new copy of the hash, with all symbols converted to strings. This is probably a better approximation of json anyway, but it also makes accessing values from the hash easier, because we always know that we'll be dealing with strings (not symbols).
Removes the duplication
No longer used, because we use stringify instead of add_indifferent_access
values added during validation, rather than replacing completely This is a fix so that the existing behaviour of insert_default is maintained (ie. anything that is already in the object is left as it was), but still allowing us to use a stringified hash internally and not modifyingt the default proc of the data object.
35dd7f6
to
e96b34f
Compare
👍 ✖️💯 |
That's a lot of thumbs. I can't compete with that. |
Replaced #add_indifferent_access with #stringify
This is a fix for #96
Previously, add_indifferent_access would modify the data and schema hashes in place, to make them accept strings or hashes as keys. However, there are two problems with this:
To resolve this, I've removed add_indifferent_access in favour of stringify, which returns a new copy of the hash, with all symbols converted to strings. This is probably a better approximation of json anyway, but it also makes accessing values from the hash easier, because we always know that we'll be dealing with strings (not symbols).