Move the attachment getter to the HasAttachedFile class

lib/paperclip.rb

@@ -174,7 +174,7 @@ module Paperclip
     #     end
     #     end
     #   end
     #   end
     def has_attached_file(name, options = {})
     def has_attached_file(name, options = {})
-      HasAttachedFile.new(name, options).define_on(self)
+      HasAttachedFile.define_on(self, name, options)
 
 
       options = Paperclip::AttachmentOptions.new(options)
       options = Paperclip::AttachmentOptions.new(options)
       Paperclip.classes_with_attachments << self.name
       Paperclip.classes_with_attachments << self.name
@@ -193,22 +193,6 @@ module Paperclip
         attachment.send(:flush_errors)
         attachment.send(:flush_errors)
       end
       end
 
 
-      define_method name do |*args|
-        ivar = "@attachment_#{name}"
-        attachment = instance_variable_get(ivar)
-
-        if attachment.nil?
-          attachment = Attachment.new(name, self, options)
-          instance_variable_set(ivar, attachment)
-        end
-
-        if args.length > 0
-          attachment.to_s(args.first)
-        else
-          attachment
-        end
-      end
-
       define_method "#{name}?" do
       define_method "#{name}?" do
         send(name).file?
         send(name).file?
       end
       end

lib/paperclip/has_attached_file.rb

 module Paperclip
 module Paperclip
   class HasAttachedFile
   class HasAttachedFile
-    def initialize(name, options)
+    def self.define_on(klass, name, options)
+      new(klass, name, options).define
+    end
+
+    def initialize(klass, name, options)
+      @klass = klass
       @name = name
       @name = name
+      @options = options
+    end
+
+    def define
+      define_getter
+      define_setter
     end
     end
 
 
-    def define_on(klass)
+    private
+
+    def define_getter
       name = @name
       name = @name
-      klass.send :define_method, "#{@name}=" do |file|
+      options = @options
+
+      @klass.send :define_method, @name do |*args|
+        ivar = "@attachment_#{name}"
+        attachment = instance_variable_get(ivar)
+
+        if attachment.nil?
+          attachment = Attachment.new(name, self, options)
+          instance_variable_set(ivar, attachment)
+        end
+
+        if args.length > 0
+          attachment.to_s(args.first)
+        else
+          attachment
+        end
+      end
+    end
+
+    def define_setter
+      name = @name
+
+      @klass.send :define_method, "#{@name}=" do |file|
         send(name).assign(file)
         send(name).assign(file)
       end
       end
     end
     end

test/has_attached_file_test.rb

@@ -4,13 +4,35 @@ require 'paperclip/has_attached_file'
 class HasAttachedFileTest < Test::Unit::TestCase
 class HasAttachedFileTest < Test::Unit::TestCase
   context '#define_on' do
   context '#define_on' do
     should 'define a setter on the class object' do
     should 'define a setter on the class object' do
+      assert_adding_attachment('avatar').defines_method('avatar=')
+    end
+
+    should 'define a getter on the class object' do
+      assert_adding_attachment('avatar').defines_method('avatar')
+    end
+  end
+
+  private
+
+  def assert_adding_attachment(attachment_name)
+    AttachmentAdder.new(attachment_name)
+  end
+
+  class AttachmentAdder
+    include Mocha::API
+    include Test::Unit::Assertions
+
+    def initialize(attachment_name)
+      @attachment_name = attachment_name
+    end
+
+    def defines_method(method_name)
       a_class = stub('class', define_method: nil)
       a_class = stub('class', define_method: nil)
-      has_attached_file = Paperclip::HasAttachedFile.new(:avatar, {})
 
 
-      has_attached_file.define_on(a_class)
+      Paperclip::HasAttachedFile.define_on(a_class, @attachment_name, {})
 
 
       assert_received(a_class, :define_method) do |expect|
       assert_received(a_class, :define_method) do |expect|
-        expect.with('avatar=')
+        expect.with(method_name)
       end
       end
     end
     end
   end
   end