Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Replace instance variables in AllowValueMatcher

  • Loading branch information...
commit 5da9f52134f1103cbedbaf00b411a3c96fc7cd40 1 parent 73723cd
@mxie mxie authored
Showing with 28 additions and 25 deletions.
  1. +28 −25 lib/shoulda/matchers/active_model/allow_value_matcher.rb
View
53 lib/shoulda/matchers/active_model/allow_value_matcher.rb
@@ -30,38 +30,40 @@ def allow_value(*values)
class AllowValueMatcher # :nodoc:
include Helpers
+ attr_accessor :options
+
def initialize(*values)
- @values_to_match = values
- @message_finder_factory = ValidationMessageFinder
- @options = {}
+ self.values_to_match = values
+ self.message_finder_factory = ValidationMessageFinder
+ self.options = {}
end
def for(attribute)
- @attribute = attribute
+ self.attribute = attribute
self
end
def with_message(message)
- @options[:expected_message] = message
+ self.options[:expected_message] = message
self
end
def strict
- @message_finder_factory = ExceptionMessageFinder
+ self.message_finder_factory = ExceptionMessageFinder
self
end
def matches?(instance)
- @instance = instance
+ self.instance = instance
- @values_to_match.all? do |current_value|
+ values_to_match.all? do |current_value|
set_attribute_on_instance(current_value)
matches_attribute_value?(current_value) && errors_do_not_match?
end
end
def failure_message_for_should
- "Did not expect #{expectation}, got error: #{@matched_error}"
+ "Did not expect #{expectation}, got error: #{matched_error}"
end
def failure_message_for_should_not
@@ -74,15 +76,16 @@ def description
private
- attr_accessor :value
+ attr_accessor :values_to_match, :message_finder_factory,
+ :instance, :attribute, :value, :matched_error
def set_attribute_on_instance(current_value)
self.value = current_value
- @instance.send("#{@attribute}=", current_value)
+ instance.send("#{attribute}=", current_value)
end
def matches_attribute_value?(current_value)
- @instance.send(@attribute.to_sym) == current_value
+ instance.send(attribute.to_sym) == current_value
end
def errors_do_not_match?
@@ -95,7 +98,7 @@ def has_no_messages?
def errors_for_attribute_match?
if expected_message
- @matched_error = errors_match_regexp? || errors_match_string?
+ self.matched_error = errors_match_regexp? || errors_match_string?
else
errors_for_attribute.compact.any?
end
@@ -119,7 +122,7 @@ def errors_match_string?
def expectation
includes_expected_message = expected_message ? "to include #{expected_message.inspect}" : ''
- [error_source, includes_expected_message, "when #{@attribute} is set to #{value.inspect}"].join(' ')
+ [error_source, includes_expected_message, "when #{attribute} is set to #{value.inspect}"].join(' ')
end
def error_source
@@ -131,19 +134,19 @@ def error_description
end
def allowed_values
- if @values_to_match.length > 1
- "any of [#{@values_to_match.map(&:inspect).join(', ')}]"
+ if values_to_match.length > 1
+ "any of [#{values_to_match.map(&:inspect).join(', ')}]"
else
- @values_to_match.first.inspect
+ values_to_match.first.inspect
end
end
def expected_message
- if @options.key?(:expected_message)
- if Symbol === @options[:expected_message]
+ if options.key?(:expected_message)
+ if Symbol === options[:expected_message]
default_expected_message
else
- @options[:expected_message]
+ options[:expected_message]
end
end
end
@@ -154,19 +157,19 @@ def default_expected_message
def default_attribute_message
default_error_message(
- @options[:expected_message],
+ options[:expected_message],
:model_name => model_name,
- :instance => @instance,
- :attribute => @attribute
+ :instance => instance,
+ :attribute => attribute
)
end
def model_name
- @instance.class.to_s.underscore
+ instance.class.to_s.underscore
end
def message_finder
- @message_finder_factory.new(@instance, @attribute)
+ message_finder_factory.new(instance, attribute)
end
end
end

2 comments on commit 5da9f52

@gabebw
Owner

What's the thinking behind this? Why not use instance variables? I believe self.thing = "foo" is the exact same as @thing = "foo" when there's an accessor.

@mxie
Owner

I think it makes sense for us to remove uses of instance variables and use barewords instead to make their implementations transparent. However, I do think that for assignments, you make a good point that we could just use @ instead of self.. I'm willing to make the change, but I'd like to see if there are any other thoughts about this.

Please sign in to comment.
Something went wrong with that request. Please try again.