Browse files

Extract default value logic into a separate class

  • Loading branch information...
1 parent be1a265 commit e8fc394cc9ce457dd82aaa01c2a0bc05d87c48d1 @solnic committed Jul 31, 2011
View
1 lib/virtus.rb
@@ -51,6 +51,7 @@ def self.included(descendant)
require 'virtus/coercion/string'
require 'virtus/coercion/symbol'
+require 'virtus/attribute/default_value'
require 'virtus/attribute'
require 'virtus/attribute/object'
require 'virtus/attribute/array'
View
32 lib/virtus/attribute.rb
@@ -96,8 +96,8 @@ def initialize(name, options = {})
@instance_variable_name = "@#{@name}".freeze
@coercion_method = @options.fetch(:coercion_method)
+ @default = DefaultValue.new(self, @options[:default])
- set_default
set_visibility
end
@@ -130,7 +130,7 @@ def get(instance)
if instance.instance_variable_defined?(instance_variable_name)
get!(instance)
else
- set_default_value(instance)
+ set!(instance, default.evaluate(instance))
end
end
@@ -223,18 +223,6 @@ def define_writer_method(mod)
private
- # Sets a default value
- #
- # @param [Object]
- #
- # @return [Object]
- # default value that was set
- #
- # @api private
- def set_default_value(instance)
- set!(instance, default.kind_of?(Proc) ? default.call(instance, self) : default)
- end
-
# Sets visibility of reader/write methods based on the options hash
#
# @return [undefined]
@@ -246,21 +234,5 @@ def set_visibility
@writer_visibility = @options.fetch(:writer, default_accessor)
end
- # Sets default value option
- #
- # @return [Object]
- #
- # @api private
- def set_default
- default = @options[:default]
-
- @default = case default
- when Proc, ::NilClass, ::TrueClass, ::FalseClass, ::Numeric, ::Symbol
- default
- else
- default.dup
- end
- end
-
end # class Attribute
end # module Virtus
View
63 lib/virtus/attribute/default_value.rb
@@ -0,0 +1,63 @@
+module Virtus
+ class Attribute
+
+ # Class representing the default value option
+ class DefaultValue
+ DUP_CLASSES = [ ::NilClass, ::TrueClass, ::FalseClass,
+ ::Numeric, ::Symbol ].freeze
+
+ # @api private
+ attr_reader :attribute
+
+ # @api private
+ attr_reader :value
+
+ # Initializes an default value instance
+ #
+ # @param [Virtus::Attribute] attribute
+ # @param [Object] value
+ #
+ # @return [undefined]
+ #
+ # @api private
+ def initialize(attribute, value)
+ @attribute = attribute
+ @value = case value when *DUP_CLASSES then value else value.dup end
@dkubb
Collaborator
dkubb added a note Jul 31, 2011

@solnic this is a bit weird. I'm trying to think of a simpler way to write it. I think the original case statement was nicer, although I would probably put it into a method like #initialize_value and have it return something you assign to @value here.

(that's a convention I'd been using to keep #initialize simple, making methods named #initialize_{ivar_name} that return the value that I assign to the ivar inside #initialize.. I don't yet know if I'm going to stick with it, but it seems to make some things a big nicer.

@solnic
Owner
solnic added a note Jul 31, 2011

@dkubb I agree, it is weird :) maybe a multi-line version in a separate method is the way to go with this.

BTW - remember that idea of having options represented by a new hierarchy of classes? I'm thinking about something like Virtus::Attribute::Option where DefaultValue < Option and Option is an abstract class with basic functionality of an option (so it has a name and a value, for a good start). It seems like a nice solution although at this moment I can't think of any good use case except that DefaultValue class cause all the other options would not have any functionality of their own.

@dkubb
Collaborator
dkubb added a note Jul 31, 2011

@solnic heh, I saw some similarities between Virtus attributes and options. Effectively we're making class level attributes, maybe not with coercion, but we're adding accessor/mutator combo methods, and setting state inside the classes.

@dkubb
Collaborator
dkubb added a note Jul 31, 2011

@solnic actually given a bit of thought I think it's cool you made DefaultValue, but I don't know if there's much point in making other Option classes, unless you think it'll move a responsibility out of a class where it doesn't belong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ end
+
+ # Evaluates the value
+ #
+ # @param [Object]
+ #
+ # @return [Object] evaluated value
+ #
+ # @api private
+ def evaluate(instance)
@dkubb
Collaborator
dkubb added a note Jul 31, 2011

I think you should make it so that the public method is #call instead of #evaluate. The calling code shouldn't care if value is callable, but it can just treat your DefaultValue instance as callable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ callable? ? call(instance) : value
+ end
+
+ private
+
+ # Evaluates a proc value
+ #
+ # @param [Object]
+ #
+ # @return [Object] evaluated value
+ #
+ # @api private
+ def call(instance)
+ value.call(instance, attribute)
+ end
+
+ # Returns if the value is callable
+ #
+ # @return [TrueClass,FalseClass]
+ #
+ # @api private
+ def callable?
+ value.respond_to?(:call)
+ end
+
+ end # class DefaultValue
+ end # class Attribute
+end # module Virtus
View
2 spec/unit/shared/attribute/get.rb
@@ -27,7 +27,7 @@
end
let(:evaluated_proc_value) do
- attribute.default.call(instance, attribute)
+ attribute.default.evaluate(attribute)
end
it { should == evaluated_proc_value }
View
24 spec/unit/virtus/attribute/default_value/class_methods/new_spec.rb
@@ -0,0 +1,24 @@
+require 'spec_helper'
+
+describe Virtus::Attribute::DefaultValue, '.new' do
+ subject { described_class.new(attribute, value) }
+
+ let(:attribute) { Virtus::Attribute::String.new(:attribute) }
+
+ context 'with a value that can be duped' do
+ let(:value) { 'something' }
+
+ its(:value) { should eql(value) }
+ its(:value) { should_not equal(value) }
+ end
+
+ context 'with a value that cannot be duped' do
+ [ nil, true, false, 1, :symbol ].each do |value|
+ context "with #{value.inspect}" do
+ let(:value) { value }
+
+ its(:value) { should equal(value) }
+ end
+ end
+ end
+end
View
19 spec/unit/virtus/attribute/default_value/instance_methods/evaluate_spec.rb
@@ -0,0 +1,19 @@
+require 'spec_helper'
+
+describe Virtus::Attribute::DefaultValue, '#evaluate' do
+ subject { object.evaluate(instance) }
+
+ let(:object) { described_class.new(attribute, value) }
+ let(:attribute) { Virtus::Attribute::String.new(:title) }
+ let(:instance) { Class.new }
+
+ context 'with a non-callable value' do
+ let(:value) { 'something' }
+ it { should eql(value) }
+ end
+
+ context 'with a callable value' do
+ let(:value) { lambda { |instance, attribute| attribute.name } }
+ it { should be(:title) }
+ end
+end

0 comments on commit e8fc394

Please sign in to comment.