Fixed a dimension interning bug

Rakefile

@@ -10,14 +10,14 @@ require 'yard'
 desc "Run specs"
 Spec::Rake::SpecTask.new('spec') do |t|
   #t.spec_files = FileList['spec/unit.spec','spec/dimension.spec','spec/quantity.spec']
-  t.spec_files = FileList['spec/dimension.spec', 'spec/unit.spec']
+  t.spec_files = FileList['spec/dimension.spec', 'spec/unit.spec', 'spec/systems.spec']
   t.spec_opts = ["-cfn"]
 end
 
 desc "specs with backtrace"
 Spec::Rake::SpecTask.new('tracespec') do |t|
   #t.spec_files = FileList['spec/unit.spec','spec/dimension.spec','spec/quantity.spec']
-  t.spec_files = FileList['spec/dimension.spec', 'spec/unit.spec']
+  t.spec_files = FileList['spec/dimension.spec', 'spec/unit.spec', 'spec/systems.spec']
   t.spec_opts = ["-bcfn"]
 end
 

lib/quantity/dimension.rb

@@ -24,7 +24,12 @@ class Dimension
     def self.for(to)
       case to
         when Dimension
-          @@dimensions[to.name]
+          if @@dimensions[to.name]
+            @@dimensions[to.name]
+          else
+            add_alias to, to.name
+            to
+          end
         when Symbol
           if @@dimensions.has_key?(to)
             @@dimensions[to]
@@ -45,9 +50,15 @@ def self.for(to)
     # @param [Symbol] name
     # @param [[Symbol String]] *aliases
     def self.add_dimension(name, *aliases)
-      dim = self.for(name) ? self.for(name) : self.new({ :name => aliases.first , :description => name})
-      self.add_alias(dim,name)
-      self.add_alias(dim,dim.string_form.to_sym,*aliases)
+      dim = nil
+      if name.is_a?(Dimension) 
+        name.name = aliases.first if aliases.first
+        dim = name
+      else
+        dim = self.for(name) || self.new({ :name => aliases.first , :description => name})
+        self.add_alias(dim,name)
+      end
+      self.add_alias(dim,*aliases)
       dim
     end
 
@@ -56,6 +67,8 @@ def self.add_dimension(name, *aliases)
     # @param [[*names]]
     def self.add_alias(dimension, *names)
       names.each do |name|
+        #puts "adding dimension alias, #{name.inspect} for #{dimension.inspect}"
+        raise ArgumentError, "Invalid dimension alias: #{name}" unless (name.to_s =~ /^(\^|\/)/).nil?
         @@dimensions[name] = dimension
       end
     end
@@ -77,7 +90,9 @@ def inspect
         "#{dimension.inspect}^#{power}"
       end
     end
-    attr_reader :numerators, :denominators, :name
+
+    attr_reader :numerators, :denominators
+    attr_accessor :name
     ### Instance-level methods/vars
 
     # A new dimension
@@ -88,10 +103,11 @@ def initialize(opts)
         (@numerators,@denominators) = Dimension.parse_string_form(opts[:description])
       elsif (opts[:numerators])
         @numerators = opts[:numerators]
-        @denominators = opts[:denominators] || []
+        @denominators = opts[:denominators]
       else
         raise ArgumentError, "Invalid options for dimension constructors"
       end
+      raise ArgumentError, "Dimensions require a numerator" unless @numerators.first.dimension
       @name = (self.class == Dimension) ? (opts[:name] || string_form.to_sym) : self.class.name.downcase.to_sym
       Dimension.add_alias(self,@name)
       Dimension.add_alias(self,string_form.to_sym)
@@ -116,9 +132,9 @@ def *(other)
     # @result [Dimension::Compound] 
     def /(other)
       raise ArgumentError, "Cannot divide #{self} by #{other.class}" unless other.is_a?(Dimension)
-      reciprocal = Dimension.for([other.denominators,other.numerators])
-      reciprocal ||= Dimension.new({:numerators => other.denominators, :denominators => other.numerators})
-      self * reciprocal
+      (new_n, new_d) = Dimension.reduce(@numerators + other.denominators, @denominators + other.numerators)
+      existing = Dimension.for([new_n,new_d])
+      existing.nil? ? Dimension.new({:numerators => new_n, :denominators => new_d}) : existing
     end
 
     # Dimensional exponentiation

spec/dimension.spec

@@ -24,12 +24,17 @@ describe Quantity::Dimension do
     Acceleration.add_dimension :'length/time^2'
     Quantity::Dimension.add_dimension :mass
     Quantity::Dimension.add_dimension :time
-    Quantity::Dimension.add_dimension :'mass*length/time^2', :force
 
     length = Quantity::Dimension.for(:length)
     mass = Quantity::Dimension.for(:mass)
     area = Quantity::Dimension.for(:area)
     accel = Quantity::Dimension.for(:acceleration)
+    time = Quantity::Dimension.for(:time)
+
+    ml = Quantity::Dimension.add_dimension mass * length
+    t2 = Quantity::Dimension.add_dimension time**2
+    Quantity::Dimension.add_dimension ml / t2, :force
+    #Quantity::Dimension.add_dimension :'length*mass/time^2', :force
     force = Quantity::Dimension.for(:'length*mass/time^2')
     force2 = Quantity::Dimension.for(:'mass*length/time^2')
 
@@ -46,12 +51,12 @@ describe Quantity::Dimension do
     mass.to_s.should == "mass"
     mass.name.should == :mass
     mass.class.should == Quantity::Dimension
+    Quantity::Dimension.for(:width).should equal(length)
     force.name.should == :force # note normalized reordering
     force.to_s.should == "force" # note normalized reordering
     force.string_form.should == "length*mass/time^2" # note normalized reordering
     force.class.should == Quantity::Dimension
     force2.should equal(force)
-    Quantity::Dimension.for(:width).should equal(length)
   end
 
   it "should have a name" do