DRY up the AR macro helpers
Reported by Ryan McGeary | July 28th, 2008 @ 09:09 PM
Ticket opened for tracking purposes.
Somewhat related to ticket #57, I've started working on DRYing up the AR macro helpers. This pattern is common:
object.send("#{attribute}=", v)
assert !object.valid?, "#{klass.name} allowed \"#{v}\" as a value for #{attribute}"
assert object.errors.on(attribute), "There are no errors set on #{attribute} after being set to \"#{v}\""
assert_contains(object.errors.on(attribute), message, "when set to \"#{v}\"")
I'd like to replace it with something like:
assert_bad_value(object, attribute, value, message)
The opposite pattern is also common, and there is an associated assert_good_value assertion for that too.
For now, I'm going to put these two assertions in the Private helper module until their method signature is hashed out or until there's a need to make them public
Comments and changes to this ticket
-
Ryan McGeary July 28th, 2008 @ 11:37 PM
(committed to thoughtbot/shoulda master)
It was short-sighted of me to assume I could just put the new assertions in the Private helper module. No go since they wouldn't be accessible inside shoulda blocks, so for now, they instead sit in the General module.
-
Tammer Saleh July 29th, 2008 @ 09:05 AM
Those assertions will be useful in general, so I think it makes sense
to have them publicly accessible.
-
Ryan McGeary July 29th, 2008 @ 09:52 AM
Tammer, Cool, after sleeping on it, I too am happy to leave those assertions public.
Soon after committing this stuff, I started on an experimental branch for further DRYing (rmm5t/experimental_super_dry_ar_helpers). Please review. Here's the relevant commit:
http://github.com/rmm5t/shoulda/...
In summary, it tries to re-use should_allow_values_for and should_not_allow_values_for wherever possible. Of course, this required tweaking those macros to accept an optional test name override to maintain the original behavior (the :should option).
I really like the code reduction that the refactor yields, but I'm unsure about how "wide" the function calls get after adding the :should option to the end.
Looking at it again, "allow #{attribute}" and "not allow #{attribute}" are repeated and maybe another optional :qualifier option would help. E.g.
should_allow_values_for attribute, min, :message => low_message, :should => "allow #{attribute} to be #{min}"turns into:
should_allow_values_for attribute, min, :message => low_message, :qualifier => "to be #{min}"Unfortunately, the :should option is still required for calls such as this:
should_not_allow_values_for attribute, nil, :message => message, :should => "require #{attribute} to be set"Maybe I'm just over-complicating things. I'm gonna step away from this problem for some time before taking any action. I think this deserves another set of eyes. Thoughts?
-
Ryan McGeary July 30th, 2008 @ 09:19 AM
- → Assigned user changed from “Tammer Saleh” to “Ryan McGeary”
-
Ryan McGeary July 30th, 2008 @ 10:39 PM
- → State changed from “new” to “committed”
Tammer, Forget about my experimental branch discussion. I'm canning the idea in favor of something cleaner that avoids the sweeping changes. The new idea arose while working on ticket #57. The existing should blocks are easier on the eyes and by moving some logic into the new assert_bad/good_value assertions, we can avoid the duplication that bothered me in the first place.
(Closing this ticket as the original premise is complete.)
Please Login or create a free account to add a new comment.
You can update this ticket by sending an email to from your email client. (help)
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
