Skip to content

Commit

Permalink
Clean up dependent option validation.
Browse files Browse the repository at this point in the history
We don't need the complexity of to_sentence, and it shouldn't be a bang
method.
  • Loading branch information
jonleighton committed Aug 10, 2012
1 parent 5ad7998 commit 08fb3c8
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 14 deletions.
Expand Up @@ -77,16 +77,12 @@ def define_writers
end
end

def check_valid_dependent!(dependent, valid_options)
unless valid_options.include?(dependent)
valid_options_message = valid_options.map(&:inspect).to_sentence(
words_connector: ', ', two_words_connector: ' or ', last_word_connector: ' or ')

raise ArgumentError, "The :dependent option expects either " \
"#{valid_options_message} (#{dependent.inspect})"
def validate_dependent_option(valid_options)
unless valid_options.include? options[:dependent]
raise ArgumentError, "The :dependent option must be one of #{valid_options}, but is :#{options[:dependent]}"
end

if dependent == :restrict
if options[:dependent] == :restrict
ActiveSupport::Deprecation.warn(
"The :restrict option is deprecated. Please use :restrict_with_exception instead, which " \
"provides the same functionality."
Expand Down
Expand Up @@ -72,7 +72,7 @@ def add_touch_callbacks(reflection)

def configure_dependency
if dependent = options[:dependent]
check_valid_dependent! dependent, [:destroy, :delete]
validate_dependent_option [:destroy, :delete]

method_name = "belongs_to_dependent_#{dependent}_for_#{name}"
model.send(:class_eval, <<-eoruby, __FILE__, __LINE__ + 1)
Expand Down
Expand Up @@ -19,9 +19,7 @@ def build

def configure_dependency
if dependent = options[:dependent]
check_valid_dependent! dependent, [:destroy, :delete_all, :nullify, :restrict,
:restrict_with_error, :restrict_with_exception]

validate_dependent_option [:destroy, :delete_all, :nullify, :restrict, :restrict_with_error, :restrict_with_exception]
send("define_#{dependent}_dependency_method")
model.before_destroy dependency_method_name
end
Expand Down
Expand Up @@ -25,8 +25,7 @@ def build

def configure_dependency
if dependent = options[:dependent]
check_valid_dependent! dependent, [:destroy, :delete, :nullify, :restrict, :restrict_with_error, :restrict_with_exception]

validate_dependent_option [:destroy, :delete, :nullify, :restrict, :restrict_with_error, :restrict_with_exception]
send("define_#{dependent}_dependency_method")
model.before_destroy dependency_method_name
end
Expand Down

1 comment on commit 08fb3c8

@carlosantoniodasilva
Copy link
Member

Choose a reason for hiding this comment

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

I kinda agree with to_sentence, and the bang thing was added following other similar check methods in Active Record (such as Relation#check_validity!. Anyway, I see this was further refactored later, thanks.

Please sign in to comment.