Fixes from Gabe and Mike comments

lib/paperclip/geometry.rb

@@ -4,6 +4,8 @@ module Paperclip
   class Geometry
   class Geometry
     attr_accessor :height, :width, :modifier, :orientation
     attr_accessor :height, :width, :modifier, :orientation
 
 
+    EXIF_ROTATED_ORIENTATION_VALUES = [5, 6, 7, 8]
+
     # Gives a Geometry representing the given height and width
     # Gives a Geometry representing the given height and width
     def initialize(width = nil, height = nil, modifier = nil)
     def initialize(width = nil, height = nil, modifier = nil)
       if width.is_a?(Hash)
       if width.is_a?(Hash)
@@ -19,22 +21,21 @@ module Paperclip
       end
       end
     end
     end
 
 
-    # Uses ImageMagick to determing the dimensions of a file, passed in as either a
-    # File or path.
-    # NOTE: (race cond) Do not reassign the 'file' variable inside this method as it is likely to be
-    # a Tempfile object, which would be eligible for file deletion when no longer referenced.
+    # Extracts the Geometry from a file (or path to a file)
     def self.from_file(file)
     def self.from_file(file)
       GeometryDetectorFactory.new(file).make
       GeometryDetectorFactory.new(file).make
     end
     end
 
 
-    # Parses a "WxH" formatted string, where W is the width and H is the height.
+    # Extracts the Geometry from a "WxH,O" string
+    # Where W is the width, H is the height,
+    # and O is the EXIF orientation
     def self.parse(string)
     def self.parse(string)
       GeometryParserFactory.new(string).make
       GeometryParserFactory.new(string).make
     end
     end
 
 
     # Swaps the height and width if necessary
     # Swaps the height and width if necessary
     def auto_orient
     def auto_orient
-      if [5, 6, 7, 8].include?(@orientation)
+      if EXIF_ROTATED_ORIENTATION_VALUES.include?(@orientation)
         @height, @width = @width, @height
         @height, @width = @width, @height
         @orientation -= 4
         @orientation -= 4
       end
       end

lib/paperclip/geometry_detector_factory.rb

@@ -12,10 +12,6 @@ module Paperclip
 
 
     private
     private
 
 
-    def path
-      @file.respond_to?(:path) ? @file.path : @file
-    end
-
     def geometry_string
     def geometry_string
       begin
       begin
         silence_stream(STDERR) do
         silence_stream(STDERR) do
@@ -28,6 +24,10 @@ module Paperclip
       end
       end
     end
     end
 
 
+    def path
+      @file.respond_to?(:path) ? @file.path : @file
+    end
+
     def raise_if_blank_file
     def raise_if_blank_file
       if path.blank?
       if path.blank?
         raise Errors::NotIdentifiedByImageMagickError.new("Cannot find the geometry of a file with a blank name")
         raise Errors::NotIdentifiedByImageMagickError.new("Cannot find the geometry of a file with a blank name")

lib/paperclip/geometry_parser_factory.rb

@@ -19,10 +19,6 @@ module Paperclip
     private
     private
 
 
     def match
     def match
-      @height = nil
-      @width = nil
-      @modifier = nil
-      @orientation = nil
       if actual_match = @string && @string.match(FORMAT)
       if actual_match = @string && @string.match(FORMAT)
         @width = actual_match[1]
         @width = actual_match[1]
         @height = actual_match[2]
         @height = actual_match[2]

test/geometry_parser_factory_test.rb

@@ -71,5 +71,3 @@ class GeometryParserFactoryTest < Test::Unit::TestCase
     assert_equal :correct, output
     assert_equal :correct, output
   end
   end
 end
 end
-
-

test/geometry_test.rb

@@ -50,26 +50,28 @@ class GeometryTest < Test::Unit::TestCase
     end
     end
 
 
     should "recognize an EXIF orientation and not rotate with auto_orient if not necessary" do
     should "recognize an EXIF orientation and not rotate with auto_orient if not necessary" do
-      assert @geo = Paperclip::Geometry.new(:width => 1024, :height => 768, :orientation => 1)
-      assert_equal 1024, @geo.width
-      assert_equal 768, @geo.height
+      geo = Paperclip::Geometry.new(:width => 1024, :height => 768, :orientation => 1)
+      assert geo
+      assert_equal 1024, geo.width
+      assert_equal 768, geo.height
 
 
-      @geo.auto_orient
+      geo.auto_orient
 
 
-      assert_equal 1024, @geo.width
-      assert_equal 768, @geo.height
+      assert_equal 1024, geo.width
+      assert_equal 768, geo.height
     end
     end
 
 
     should "recognize an EXIF orientation and rotate with auto_orient if necessary" do
     should "recognize an EXIF orientation and rotate with auto_orient if necessary" do
-      assert @geo = Paperclip::Geometry.new(:width => 1024, :height => 768, :orientation => 6)
-      assert_equal 1024, @geo.width
-      assert_equal 768, @geo.height
+      geo = Paperclip::Geometry.new(:width => 1024, :height => 768, :orientation => 6)
+      assert geo
+      assert_equal 1024, geo.width
+      assert_equal 768, geo.height
 
 
-      @geo.auto_orient
+      geo.auto_orient
 
 
-      assert_equal 768, @geo.width
-      assert_equal 1024, @geo.height
-      assert_equal 2, @geo.orientation
+      assert_equal 768, geo.width
+      assert_equal 1024, geo.height
+      assert_equal 2, geo.orientation
     end
     end
 
 
     should "treat x and X the same in geometries" do
     should "treat x and X the same in geometries" do