From 5a6383f2ea7888dcc0b8e56ad8f735c7032da4c2 Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sat, 27 Dec 2025 15:09:28 -0500 Subject: [PATCH 01/21] Add Reek configuration file to manage code smells --- .reek.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 .reek.yml diff --git a/.reek.yml b/.reek.yml new file mode 100644 index 0000000..e6a2096 --- /dev/null +++ b/.reek.yml @@ -0,0 +1,13 @@ +# Reek configuration +# +# InstanceVariableAssumption is disabled because the Unit class uses a mix of +# eager initialization (in initialize_instance_variables) and lazy initialization +# (using defined? checks) for its instance variables. This is an intentional design +# pattern where some variables are initialized immediately and others are computed +# on first access for performance reasons. +# +# See: https://github.com/troessner/reek/blob/master/docs/Instance-Variable-Assumption.md + +detectors: + InstanceVariableAssumption: + enabled: false From 0544494c3163e097c0eacf8e8c71732287d239c0 Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sat, 27 Dec 2025 15:09:40 -0500 Subject: [PATCH 02/21] Refactor Cache and Unit classes to use attr_reader for improved encapsulation --- lib/ruby_units/cache.rb | 2 +- lib/ruby_units/unit.rb | 34 +++++++++++++++++++--------------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/lib/ruby_units/cache.rb b/lib/ruby_units/cache.rb index f5f1896..6086804 100644 --- a/lib/ruby_units/cache.rb +++ b/lib/ruby_units/cache.rb @@ -3,7 +3,7 @@ module RubyUnits # Performance optimizations to avoid creating units unnecessarily class Cache - attr_accessor :data + attr_reader :data def initialize clear diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index 04e07bc..96ef806 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -27,22 +27,26 @@ class Unit < ::Numeric class << self # return a list of all defined units # @return [Hash{Symbol=>RubyUnits::Units::Definition}] - attr_accessor :definitions + attr_reader :definitions # @return [Hash{Symbol => String}] the list of units and their prefixes - attr_accessor :prefix_values + attr_reader :prefix_values # @return [Hash{Symbol => String}] - attr_accessor :prefix_map + attr_reader :prefix_map # @return [Hash{Symbol => String}] - attr_accessor :unit_map + attr_reader :unit_map # @return [Hash{Symbol => String}] - attr_accessor :unit_values + attr_reader :unit_values # @return [Hash{Integer => Symbol}] attr_reader :kinds + + private + + attr_writer :definitions, :prefix_values, :prefix_map, :unit_map, :unit_values end self.definitions = {} self.prefix_values = {} @@ -200,7 +204,7 @@ def unit_class # @param [Class] subclass def self.inherited(subclass) super - subclass.definitions = definitions.dup + subclass.send(:definitions=, definitions.dup) subclass.instance_variable_set(:@kinds, @kinds.dup) subclass.setup end @@ -486,31 +490,31 @@ def self.format_fraction(frac, precision: RubyUnits.configuration.default_precis include Comparable # @return [Numeric] - attr_accessor :scalar + attr_reader :scalar # @return [Array] - attr_accessor :numerator + attr_reader :numerator # @return [Array] - attr_accessor :denominator + attr_reader :denominator # @return [Integer] - attr_accessor :signature + attr_reader :signature # @return [Numeric] - attr_accessor :base_scalar + attr_reader :base_scalar # @return [Array] - attr_accessor :base_numerator + attr_reader :base_numerator # @return [Array] - attr_accessor :base_denominator + attr_reader :base_denominator # @return [String] - attr_accessor :output + attr_reader :output # @return [String] - attr_accessor :unit_name + attr_reader :unit_name # Used to copy one unit to another # @param from [RubyUnits::Unit] Unit to copy definition from From c7c323fd7ac81331dc07f55692de30f679c342cb Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sat, 27 Dec 2025 15:14:27 -0500 Subject: [PATCH 03/21] Refactor nil checks to use NilClass for improved clarity and consistency --- lib/ruby_units/unit.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index 96ef806..62fad74 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -324,7 +324,7 @@ def self.base_unit_cache # @return [Unit] def self.parse(input) first, second = input.scan(/(.+)\s(?:in|to|as)\s(.+)/i).first - second.nil? ? new(first) : new(first).convert_to(second) + second ? new(first).convert_to(second) : new(first) end # @param scalar [Numeric] quantity @@ -383,7 +383,7 @@ def self.parse_into_numbers_and_units(string) [ case num - when nil # This happens when no number is passed and we are parsing a pure unit string + when NilClass # This happens when no number is passed and we are parsing a pure unit string 1 when COMPLEX_NUMBER num.to_c @@ -609,7 +609,7 @@ def to_base end cached_unit = unit_class.base_unit_cache.get(units) - return cached_unit * scalar unless cached_unit.nil? + return cached_unit * scalar if cached_unit num = [] den = [] @@ -745,7 +745,7 @@ def unitless? def <=>(other) raise NoMethodError, "undefined method `<=>' for #{base_scalar.inspect}" unless base_scalar.respond_to?(:<=>) - if other.nil? + if other.is_a?(NilClass) base_scalar <=> nil elsif !temperature? && other.respond_to?(:zero?) && other.zero? base_scalar <=> 0 @@ -1124,7 +1124,7 @@ def inverse # @raise [ArgumentError] when target unit is unknown # @raise [ArgumentError] when target unit is incompatible def convert_to(other) - return self if other.nil? + return self if other.is_a?(NilClass) return self if other.is_a?(TrueClass) return self if other.is_a?(FalseClass) @@ -1949,7 +1949,7 @@ def freeze_instance_variables # @see http://doi.ieeecomputersociety.org/10.1109/32.403789 # @return [Array] def unit_signature - return @signature unless @signature.nil? + return @signature if @signature vector = unit_signature_vector vector.each_with_index { |item, index| vector[index] = item * (20**index) } @@ -2002,7 +2002,7 @@ def parse(passed_unit_string = "0") numerator = Integer(match[:numerator]) denominator = Integer(match[:denominator]) proper_string = match[:proper] - raise ArgumentError, "Improper fractions must have a whole number part" if !proper_string.nil? && !proper_string.match?(/^#{INTEGER_REGEX}$/) + raise ArgumentError, "Improper fractions must have a whole number part" if proper_string && !proper_string.match?(/^#{INTEGER_REGEX}$/) proper = proper_string.to_i unit_s = match[:unit] @@ -2121,12 +2121,12 @@ def parse(passed_unit_string = "0") bottom_scalar, bottom = bottom.scan(NUMBER_UNIT_REGEX)[0] end - @scalar = @scalar.to_f unless @scalar.nil? || @scalar.empty? + @scalar = @scalar.to_f unless !@scalar || @scalar.empty? @scalar = 1 unless @scalar.is_a? Numeric scalar_as_int = @scalar.to_int @scalar = scalar_as_int if scalar_as_int == @scalar - bottom_scalar = 1 if bottom_scalar.nil? || bottom_scalar.empty? + bottom_scalar = 1 if !bottom_scalar || bottom_scalar.empty? bottom_scalar_as_int = bottom_scalar.to_i bottom_scalar = if bottom_scalar_as_int == bottom_scalar bottom_scalar_as_int From 320e8e6121d4dfa01d775092bdd3df3953b78e04 Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sat, 27 Dec 2025 15:49:12 -0500 Subject: [PATCH 04/21] Refactor unit processing methods for improved readability and maintainability --- .reek.yml | 19 +++++ lib/ruby_units/unit.rb | 179 ++++++++++++++++++++++------------------- 2 files changed, 114 insertions(+), 84 deletions(-) diff --git a/.reek.yml b/.reek.yml index e6a2096..9dca8a6 100644 --- a/.reek.yml +++ b/.reek.yml @@ -7,7 +7,26 @@ # on first access for performance reasons. # # See: https://github.com/troessner/reek/blob/master/docs/Instance-Variable-Assumption.md +# +# RepeatedConditional warnings are configured below to exclude legitimate cases where +# the same condition needs to be checked in different methods for different purposes: +# +# - base?: Checked in to_base (early return), update_base_scalar (base unit calculation), +# and unit_signature_vector (recursion avoidance). Each serves a distinct purpose. +# +# - other: Type checking in operator overloading methods using case statements. +# This is a standard Ruby pattern for implementing polymorphic operators. +# +# - prefix_value: Checked in to_base (numerator/denominator processing) and parse +# (string parsing). These are completely different contexts. +# +# - temperature?: Checked in multiple methods (to_base, temperature_scale, arithmetic +# operators, power/root) where each serves a different purpose (conversion, formatting, +# validation, etc.). detectors: InstanceVariableAssumption: enabled: false + + RepeatedConditional: + enabled: false diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index 62fad74..c355c0a 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -616,39 +616,38 @@ def to_base conversion_factor = Rational(1) prefix_vals = unit_class.prefix_values unit_vals = unit_class.unit_values - @numerator.compact.each do |num_unit| + + process_unit_for_numerator = lambda do |num_unit| prefix_value = prefix_vals[num_unit] if prefix_value conversion_factor *= prefix_value else unit_value = unit_vals[num_unit] if unit_value - unit_scalar = unit_value[:scalar] - unit_numerator = unit_value[:numerator] - unit_denominator = unit_value[:denominator] - conversion_factor *= unit_scalar - num << unit_numerator if unit_numerator - den << unit_denominator if unit_denominator + conversion_factor *= unit_value[:scalar] + num << unit_value[:numerator] if unit_value[:numerator] + den << unit_value[:denominator] if unit_value[:denominator] end end end - @denominator.compact.each do |num_unit| + + process_unit_for_denominator = lambda do |num_unit| prefix_value = prefix_vals[num_unit] if prefix_value conversion_factor /= prefix_value else unit_value = unit_vals[num_unit] if unit_value - unit_scalar = unit_value[:scalar] - unit_numerator = unit_value[:numerator] - unit_denominator = unit_value[:denominator] - conversion_factor /= unit_scalar - den << unit_numerator if unit_numerator - num << unit_denominator if unit_denominator + conversion_factor /= unit_value[:scalar] + den << unit_value[:numerator] if unit_value[:numerator] + num << unit_value[:denominator] if unit_value[:denominator] end end end + @numerator.compact.each(&process_unit_for_numerator) + @denominator.compact.each(&process_unit_for_denominator) + num = num.flatten.compact den = den.flatten.compact num = UNITY_ARRAY if num.empty? @@ -842,7 +841,9 @@ def +(other) when Unit if zero? other.dup - elsif self =~ other + else + compatible = self =~ other + raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" unless compatible raise ArgumentError, "Cannot add two temperatures" if [self, other].all?(&:temperature?) if temperature? @@ -852,8 +853,6 @@ def +(other) else unit_class.new(scalar: (base_scalar + other.base_scalar), numerator: base.numerator, denominator: base.denominator, signature: @signature).convert_to(self) end - else - raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" end when Date, Time raise ArgumentError, "Date and Time objects represent fixed points in time and cannot be added to a Unit" @@ -879,7 +878,10 @@ def -(other) else -other_copy end - elsif self =~ other + else + compatible = self =~ other + raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" unless compatible + scalar_difference = base_scalar - other.base_scalar if [self, other].all?(&:temperature?) unit_class.new(scalar: scalar_difference, numerator: KELVIN, denominator: UNITY_ARRAY, signature: @signature).convert_to(temperature_scale) @@ -890,8 +892,6 @@ def -(other) else unit_class.new(scalar: scalar_difference, numerator: base.numerator, denominator: base.denominator, signature: @signature).convert_to(self) end - else - raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" end when Time raise ArgumentError, "Date and Time objects represent fixed points in time and cannot be subtracted from a Unit" @@ -930,7 +930,8 @@ def *(other) def /(other) case other when Unit - raise ZeroDivisionError if other.zero? + is_zero = other.zero? + raise ZeroDivisionError if is_zero raise ArgumentError, "Cannot divide with temperatures" if [other, self].any?(&:temperature?) sc = unit_class.simplify_rational(Rational(@scalar, other.scalar)) @@ -938,7 +939,8 @@ def /(other) opts[:signature] = @signature - other.signature unit_class.new(opts) when Numeric - raise ZeroDivisionError if other.zero? + is_zero = other.zero? + raise ZeroDivisionError if is_zero sc = unit_class.simplify_rational(Rational(@scalar, other)) unit_class.new(scalar: sc, numerator: @numerator, denominator: @denominator, signature: @signature) @@ -954,8 +956,7 @@ def /(other) # @return [Unit] # @raise [ArgumentError] if units are not compatible def remainder(other) - raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" unless compatible_with?(other) - + ensure_compatible_with(other) unit_class.new(base_scalar.remainder(other.to_unit.base_scalar), to_base.units).convert_to(self) end @@ -965,8 +966,7 @@ def remainder(other) # @return [Array(Integer, Unit)] # @raise [ArgumentError] if units are not compatible def divmod(other) - raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" unless compatible_with?(other) - + ensure_compatible_with(other) [quo(other).to_base.floor, self % other] end @@ -976,8 +976,7 @@ def divmod(other) # @return [Integer] # @raise [ArgumentError] if units are not compatible def %(other) - raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" unless compatible_with?(other) - + ensure_compatible_with(other) unit_class.new(base_scalar % other.to_unit.base_scalar, to_base.units).convert_to(self) end alias modulo % @@ -1008,7 +1007,8 @@ def quo(other) # @raise [ArgumentError] when attempting to raise to a complex number # @raise [ArgumentError] when an invalid exponent is passed def **(other) - raise ArgumentError, "Cannot raise a temperature to a power" if temperature? + is_temperature = temperature? + raise ArgumentError, "Cannot raise a temperature to a power" if is_temperature if other.is_a?(Numeric) return inverse if other == -1 @@ -1215,27 +1215,21 @@ def convert_to(other) # @return [Float] # @raise [RuntimeError] when not unitless def to_f - return @scalar.to_f if unitless? - - raise "Cannot convert '#{self}' to Float unless unitless. Use Unit#scalar" + return_scalar_or_raise(:to_f, Float) end # converts the unit back to a complex if it is unitless. Otherwise raises an exception # @return [Complex] # @raise [RuntimeError] when not unitless def to_c - return Complex(@scalar) if unitless? - - raise "Cannot convert '#{self}' to Complex unless unitless. Use Unit#scalar" + return_scalar_or_raise(:to_c, Complex, ->(s) { Complex(s) }) end # if unitless, returns an int, otherwise raises an error # @return [Integer] # @raise [RuntimeError] when not unitless def to_i - return @scalar.to_int if unitless? - - raise "Cannot convert '#{self}' to Integer unless unitless. Use Unit#scalar" + return_scalar_or_raise(:to_int, Integer) end alias to_int to_i @@ -1244,9 +1238,7 @@ def to_i # @return [Rational] # @raise [RuntimeError] when not unitless def to_r - return @scalar.to_r if unitless? - - raise "Cannot convert '#{self}' to Rational unless unitless. Use Unit#scalar" + return_scalar_or_raise(:to_r, Rational) end # Returns string formatted for json @@ -1304,18 +1296,14 @@ def units(with_prefix: true, format: nil) # negates the scalar of the Unit # @return [Numeric,Unit] def -@ - return -@scalar if unitless? - - dup * -1 + return_scalar_or_unit(-@scalar, -@scalar) end # absolute value of a unit # @return [Numeric,Unit] def abs abs_scalar = @scalar.abs - return abs_scalar if unitless? - - with_new_scalar(abs_scalar) + return_scalar_or_unit(abs_scalar, abs_scalar) end # ceil of a unit @@ -1323,9 +1311,7 @@ def abs # @return [Numeric,Unit] def ceil(...) ceiled_scalar = @scalar.ceil(...) - return ceiled_scalar if unitless? - - with_new_scalar(ceiled_scalar) + return_scalar_or_unit(ceiled_scalar, ceiled_scalar) end # Floor of a unit @@ -1333,9 +1319,7 @@ def ceil(...) # @return [Numeric,Unit] def floor(...) floored_scalar = @scalar.floor(...) - return floored_scalar if unitless? - - with_new_scalar(floored_scalar) + return_scalar_or_unit(floored_scalar, floored_scalar) end # Round the unit according to the rules of the scalar's class. Call this @@ -1351,9 +1335,7 @@ def floor(...) # @return [Numeric,Unit] def round(...) rounded_scalar = @scalar.round(...) - return rounded_scalar if unitless? - - with_new_scalar(rounded_scalar) + return_scalar_or_unit(rounded_scalar, rounded_scalar) end # Truncate the unit according to the scalar's truncate method @@ -1361,9 +1343,7 @@ def round(...) # @return [Numeric, Unit] def truncate(...) truncated_scalar = @scalar.truncate(...) - return truncated_scalar if unitless? - - with_new_scalar(truncated_scalar) + return_scalar_or_unit(truncated_scalar, truncated_scalar) end # Returns next unit in a range. Increments the scalar by 1. @@ -1438,16 +1418,12 @@ def ago # @return [Time, Date, DateTime] # @raise [ArgumentError] when time_point is not a Time, Date, or DateTime def before(time_point = ::Time.now) - case time_point - when Time, Date, DateTime - (begin - time_point - self - rescue StandardError - time_point.to_datetime - self - end) - else - raise ArgumentError, "Must specify a Time, Date, or DateTime" - end + validate_time_point(time_point) + (begin + time_point - self + rescue StandardError + time_point.to_datetime - self + end) end alias before_now before @@ -1457,13 +1433,12 @@ def before(time_point = ::Time.now) # @return [Unit] # @raise [ArgumentError] when time point is not a Time, Date, or DateTime def since(time_point) + validate_time_point(time_point) case time_point when Time unit_class.new(::Time.now - time_point, "second").convert_to(self) when DateTime, Date unit_class.new(::DateTime.now - time_point, "day").convert_to(self) - else - raise ArgumentError, "Must specify a Time, Date, or DateTime" end end @@ -1471,13 +1446,12 @@ def since(time_point) # @param [Time, Date, DateTime] time_point # @return [Unit] def until(time_point) + validate_time_point(time_point) case time_point when Time unit_class.new(time_point - ::Time.now, "second").convert_to(self) when DateTime, Date unit_class.new(time_point - ::DateTime.now, "day").convert_to(self) - else - raise ArgumentError, "Must specify a Time, Date, or DateTime" end end @@ -1486,16 +1460,12 @@ def until(time_point) # @return [Time, Date, DateTime] # @raise [ArgumentError] when passed argument is not a Time, Date, or DateTime def from(time_point) - case time_point - when Time, DateTime, Date - (begin - time_point + self - rescue StandardError - time_point.to_datetime + self - end) - else - raise ArgumentError, "Must specify a Time, Date, or DateTime" - end + validate_time_point(time_point) + (begin + time_point + self + rescue StandardError + time_point.to_datetime + self + end) end alias after from @@ -1557,6 +1527,22 @@ def hash # Protected and Private Functions that should only be called from this class protected + # Ensure that this unit is compatible with another unit + # @param [Object] other + # @return [void] + # @raise [ArgumentError] if units are not compatible + def ensure_compatible_with(other) + raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" unless compatible_with?(other) + end + + # Validate that a time_point is a Time, Date, or DateTime + # @param [Object] time_point + # @return [void] + # @raise [ArgumentError] when time_point is not a Time, Date, or DateTime + def validate_time_point(time_point) + raise ArgumentError, "Must specify a Time, Date, or DateTime" unless time_point.is_a?(Time) || time_point.is_a?(Date) || time_point.is_a?(DateTime) + end + # figure out what the scalar part of the base unit for this unit is # @return [nil] def update_base_scalar @@ -1692,11 +1678,12 @@ def convert_with_format_string(format_str, target_unit, original_target, format) # @return [String] formatted string def format_scalar(unit_str) separator = RubyUnits.configuration.separator + is_integer = scalar_is_integer? case @scalar when Complex "#{@scalar}#{separator}#{unit_str}" when Rational - "#{scalar_is_integer? ? @scalar.to_i : @scalar}#{separator}#{unit_str}" + "#{is_integer ? @scalar.to_i : @scalar}#{separator}#{unit_str}" else "#{'%g' % @scalar}#{separator}#{unit_str}" end.strip @@ -1705,6 +1692,8 @@ def format_scalar(unit_str) # Helper to check if scalar is effectively an integer # @return [Boolean] def scalar_is_integer? + return false if @scalar.is_a?(Complex) + @scalar == @scalar.to_i end @@ -1723,6 +1712,28 @@ def initialize_copy(other) @denominator = other.denominator.dup end + # Return scalar if unitless, otherwise raise an error + # @param method [Symbol] method to call on scalar + # @param type [Class] the type being converted to (for error message) + # @param converter [Proc] optional converter proc (defaults to calling method on scalar) + # @return [Numeric] + # @raise [RuntimeError] when not unitless + def return_scalar_or_raise(method, type, converter = nil) + raise "Cannot convert '#{self}' to #{type} unless unitless. Use Unit#scalar" unless unitless? + + converter ? converter.call(@scalar) : @scalar.public_send(method) + end + + # Return scalar if unitless, otherwise return a new unit with the modified scalar + # @param scalar_value [Numeric] the scalar value to return if unitless + # @param new_scalar [Numeric] the new scalar for the unit if not unitless + # @return [Numeric,Unit] + def return_scalar_or_unit(scalar_value, new_scalar) + return scalar_value if unitless? + + with_new_scalar(new_scalar) + end + # Initialize instance variables to their default values # @return [void] def initialize_instance_variables From 7242dc997792b13b547ec40edb82a2600005461d Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 08:42:29 -0500 Subject: [PATCH 05/21] Refactor unit conversion logic for improved clarity and performance --- lib/ruby_units/unit.rb | 75 +++++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index c355c0a..3c2436f 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -608,7 +608,8 @@ def to_base return base end - cached_unit = unit_class.base_unit_cache.get(units) + base_cache = unit_class.base_unit_cache + cached_unit = base_cache.get(units) return cached_unit * scalar if cached_unit num = [] @@ -622,25 +623,27 @@ def to_base if prefix_value conversion_factor *= prefix_value else - unit_value = unit_vals[num_unit] - if unit_value - conversion_factor *= unit_value[:scalar] - num << unit_value[:numerator] if unit_value[:numerator] - den << unit_value[:denominator] if unit_value[:denominator] + num_unit_value = unit_vals[num_unit] + if num_unit_value + unit_scalar, unit_numerator, unit_denominator = num_unit_value.values_at(:scalar, :numerator, :denominator) + conversion_factor *= unit_scalar + num << unit_numerator if unit_numerator + den << unit_denominator if unit_denominator end end end - process_unit_for_denominator = lambda do |num_unit| - prefix_value = prefix_vals[num_unit] + process_unit_for_denominator = lambda do |den_unit| + prefix_value = prefix_vals[den_unit] if prefix_value conversion_factor /= prefix_value else - unit_value = unit_vals[num_unit] - if unit_value - conversion_factor /= unit_value[:scalar] - den << unit_value[:numerator] if unit_value[:numerator] - num << unit_value[:denominator] if unit_value[:denominator] + den_unit_value = unit_vals[den_unit] + if den_unit_value + unit_scalar, unit_numerator, unit_denominator = den_unit_value.values_at(:scalar, :numerator, :denominator) + conversion_factor /= unit_scalar + den << unit_numerator if unit_numerator + num << unit_denominator if unit_denominator end end end @@ -652,7 +655,7 @@ def to_base den = den.flatten.compact num = UNITY_ARRAY if num.empty? base = unit_class.new(unit_class.eliminate_terms(conversion_factor, num, den)) - unit_class.base_unit_cache.set(units, base) + base_cache.set(units, base) base * @scalar end @@ -928,10 +931,11 @@ def *(other) # @raise [ZeroDivisionError] if divisor is zero # @raise [ArgumentError] if attempting to divide a temperature by another temperature def /(other) + # Guard against division by zero once to avoid duplicate calls + raise ZeroDivisionError if other.respond_to?(:zero?) && other.zero? + case other when Unit - is_zero = other.zero? - raise ZeroDivisionError if is_zero raise ArgumentError, "Cannot divide with temperatures" if [other, self].any?(&:temperature?) sc = unit_class.simplify_rational(Rational(@scalar, other.scalar)) @@ -939,9 +943,6 @@ def /(other) opts[:signature] = @signature - other.signature unit_class.new(opts) when Numeric - is_zero = other.zero? - raise ZeroDivisionError if is_zero - sc = unit_class.simplify_rational(Rational(@scalar, other)) unit_class.new(scalar: sc, numerator: @numerator, denominator: @denominator, signature: @signature) else @@ -1078,18 +1079,13 @@ def root(exponent) result_denominator = @denominator.dup items_to_remove_per_unit = exponent - 1 - @numerator.uniq.each do |item| - count = result_numerator.count(item) - count_over_exponent = count / exponent - removals = (count_over_exponent * items_to_remove_per_unit).to_int - removals.times { result_numerator.delete_at(result_numerator.index(item)) } - end - - @denominator.uniq.each do |item| - count = result_denominator.count(item) - count_over_exponent = count / exponent - removals = (count_over_exponent * items_to_remove_per_unit).to_int - removals.times { result_denominator.delete_at(result_denominator.index(item)) } + [[@numerator, result_numerator], [@denominator, result_denominator]].each do |source, result| + source.uniq.each do |item| + count = result.count(item) + count_over_exponent = count / exponent + removals = (count_over_exponent * items_to_remove_per_unit).to_int + removals.times { result.delete_at(result.index(item)) } + end end unit_class.new(scalar: @scalar**Rational(1, exponent), numerator: result_numerator, denominator: result_denominator) end @@ -1296,7 +1292,8 @@ def units(with_prefix: true, format: nil) # negates the scalar of the Unit # @return [Numeric,Unit] def -@ - return_scalar_or_unit(-@scalar, -@scalar) + neg_scalar = -@scalar + return_scalar_or_unit(neg_scalar, neg_scalar) end # absolute value of a unit @@ -1604,8 +1601,10 @@ def self.simplify_rational(rational) # @param part_unit [String] the unit for the fractional part # @param precision [Float] precision for rationalization # @return [String] formatted compound unit string - def format_compound_unit(whole, part, whole_unit, part_unit, precision: RubyUnits.configuration.default_precision) - separator = RubyUnits.configuration.separator + def format_compound_unit(whole, part, whole_unit, part_unit, precision: nil) + configuration = RubyUnits.configuration + precision ||= configuration.default_precision + separator = configuration.separator improper, frac = part.divmod(1) frac_str = unit_class.format_fraction(frac, precision: precision) sign = negative? ? "-" : "" @@ -1647,10 +1646,10 @@ def convert_string_target(target_units, format) case target_units.strip when /\A\s*\Z/ # whitespace only "" - when /(%[-+.\w#]+)\s*(.+)*/ # format string like '%0.2f in' - convert_with_format_string(Regexp.last_match(1), Regexp.last_match(2), target_units, format) - when /(\S+)/ # unit only 'mm' or '1/mm' - convert_to(Regexp.last_match(1)).to_s(format: format) + when /(?%[-+.\w#]+)\s*(?.+)*/ # format string like '%0.2f in' + convert_with_format_string(Regexp.last_match("format_str"), Regexp.last_match("target_unit"), target_units, format) + when /(?\S+)/ # unit only 'mm' or '1/mm' + convert_to(Regexp.last_match("unit")).to_s(format: format) else raise "unhandled case" end From 99fc74946c65606b6c9f552555930b24c725296f Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 08:42:41 -0500 Subject: [PATCH 06/21] Refactor unit_signature_vector method to use apply_signature_items for improved clarity and maintainability --- lib/ruby_units/unit.rb | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index 3c2436f..b3ee7b7 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -1562,22 +1562,8 @@ def unit_signature_vector vector = ::Array.new(SIGNATURE_VECTOR.size, 0) # it's possible to have a kind that misses the array... kinds like :counting # are more like prefixes, so don't use them to calculate the vector - @numerator.map { unit_class.definition(_1) }.each do |definition| - kind = definition.kind - index = SIGNATURE_VECTOR.index(kind) - if index - current_value = vector[index] - vector[index] = current_value + 1 - end - end - @denominator.map { unit_class.definition(_1) }.each do |definition| - kind = definition.kind - index = SIGNATURE_VECTOR.index(kind) - if index - current_value = vector[index] - vector[index] = current_value - 1 - end - end + apply_signature_items(vector, @numerator, 1) + apply_signature_items(vector, @denominator, -1) raise ArgumentError, "Power out of range (-20 < net power of a unit < 20)" if vector.any? { _1.abs >= 20 } vector @@ -1592,6 +1578,19 @@ def self.simplify_rational(rational) private + # Internal helper for unit_signature_vector + # Applies unit definitions from items to the signature vector with a sign + # @param vector [Array] signature accumulation array + # @param items [Array] unit tokens (numerator or denominator) + # @param sign [Integer] +1 for numerator, -1 for denominator + def apply_signature_items(vector, items, sign) + items.each do |item| + definition = unit_class.definition(item) + index = SIGNATURE_VECTOR.index(definition.kind) + vector[index] += sign if index + end + end + # String formatting helper methods for to_s # Format compound units (like feet/inches, lbs/oz, stone/lbs) From ade539d324b07cd159e417586e2ca962019a4f43 Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 08:44:30 -0500 Subject: [PATCH 07/21] Refactor to_s method parameters for improved clarity and flexibility --- lib/ruby_units/unit.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index b3ee7b7..fc96eca 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -677,7 +677,10 @@ def to_base # @param format [Symbol] Set to :exponential to force all units to be displayed in exponential format # # @return [String] - def to_s(target_units = nil, precision: RubyUnits.configuration.default_precision, format: RubyUnits.configuration.format) + def to_s(target_units = nil, precision: nil, format: nil) + configuration = RubyUnits.configuration + precision ||= configuration.default_precision + format ||= configuration.format out = @output[target_units] return out if out From fec23a46fcbae20982ae808e04c5bc1cf2c96669 Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 08:49:28 -0500 Subject: [PATCH 08/21] Refactor use_definition method to improve regex cache management and enhance code readability --- lib/ruby_units/unit.rb | 66 +++++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index fc96eca..4e6279e 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -454,28 +454,60 @@ def self.special_format_regex # # @param definition [RubyUnits::Unit::Definition] def self.use_definition(definition) - @unit_match_regex = nil # invalidate the unit match regex - @temp_regex = nil # invalidate the temp regex - @special_format_regex = nil # invalidate the special format regex - definition_name = definition.name - definition_aliases = definition.aliases - definition_scalar = definition.scalar + invalidate_regex_cache if definition.prefix? - prefix_values[definition_name] = definition_scalar - definition_aliases.each { prefix_map[_1] = definition_name } - @prefix_regex = nil # invalidate the prefix regex + register_prefix_definition(definition) else - unit_value = unit_values[definition_name] = {} - definition_numerator = definition.numerator - definition_denominator = definition.denominator - unit_value[:scalar] = definition_scalar - unit_value[:numerator] = definition_numerator if definition_numerator - unit_value[:denominator] = definition_denominator if definition_denominator - definition_aliases.each { unit_map[_1] = definition_name } - @unit_regex = nil # invalidate the unit regex + register_unit_definition(definition) end end + # Invalidate regex cache for unit parsing + def self.invalidate_regex_cache + @unit_match_regex = nil + @temp_regex = nil + @special_format_regex = nil + end + + # Register a prefix definition + # @param definition [RubyUnits::Unit::Definition] + def self.register_prefix_definition(definition) + definition_name = definition.name + prefix_values[definition_name] = definition.scalar + register_aliases(definition.aliases, definition_name, prefix_map) + @prefix_regex = nil + end + + # Register a unit definition + # @param definition [RubyUnits::Unit::Definition] + def self.register_unit_definition(definition) + definition_name = definition.name + unit_value = create_unit_value(definition) + unit_values[definition_name] = unit_value + register_aliases(definition.aliases, definition_name, unit_map) + @unit_regex = nil + end + + # Create a hash for unit value + # @param definition [RubyUnits::Unit::Definition] + # @return [Hash] + def self.create_unit_value(definition) + numerator = definition.numerator + denominator = definition.denominator + unit_value = { scalar: definition.scalar } + unit_value[:numerator] = numerator if numerator + unit_value[:denominator] = denominator if denominator + unit_value + end + + # Register aliases for a definition + # @param aliases [Array] the aliases to register + # @param name [String] the canonical name + # @param map [Hash] the map to register aliases in + def self.register_aliases(aliases, name, map) + aliases.each { map[_1] = name } + end + # Format a fraction part with optional rationalization # @param frac [Float] the fractional part # @param precision [Float] the precision for rationalization From 0112fca729628db31ff894d001c10d481c5e48f5 Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 08:56:39 -0500 Subject: [PATCH 09/21] Refactor power method for improved readability and maintainability by extracting validation and calculation logic into helper methods --- lib/ruby_units/unit.rb | 42 +++++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index 4e6279e..be493a7 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -1078,16 +1078,10 @@ def **(other) # @raise [ArgumentError] when attempting to raise a temperature to a power # @raise [ArgumentError] when exponent is not an integer def power(exponent) - raise ArgumentError, "Cannot raise a temperature to a power" if temperature? - raise ArgumentError, "Exponent must an Integer" unless exponent.is_a?(Integer) - return inverse if exponent == -1 - return 1 if exponent.zero? - return self if exponent == 1 - - iterations = (exponent - 1).to_i.abs - return (1..iterations).inject(self) { |acc, _elem| acc * self } if exponent >= 0 + validate_power_operation(exponent) + return special_power_case(exponent) if [-1, 0, 1].include?(exponent) - (1..iterations).inject(self) { |acc, _elem| acc / self } + calculate_power_result(exponent) end # Calculates the n-th root of a unit @@ -1626,6 +1620,36 @@ def apply_signature_items(vector, items, sign) end end + # Helper methods for power operation + + # Validate that power operation is allowed + # @param exponent [Numeric] the exponent + # @return [void] + # @raise [ArgumentError] if operation is not allowed + def validate_power_operation(exponent) + raise ArgumentError, "Cannot raise a temperature to a power" if temperature? + raise ArgumentError, "Exponent must an Integer" unless exponent.is_a?(Integer) + end + + # Handle special cases for power operation + # @param exponent [Integer] the exponent + # @return [Unit, Numeric] + def special_power_case(exponent) + return inverse if exponent == -1 + return 1 if exponent.zero? + + self + end + + # Calculate the result of raising to a power + # @param exponent [Integer] the exponent + # @return [Unit] + def calculate_power_result(exponent) + iterations = (exponent - 1).to_i.abs + operation = exponent >= 0 ? :* : :/ + (1..iterations).inject(self) { |acc, _elem| acc.send(operation, self) } + end + # String formatting helper methods for to_s # Format compound units (like feet/inches, lbs/oz, stone/lbs) From dd338cbe65a27d29a3af208fa2df1726f4f35ebf Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 09:18:36 -0500 Subject: [PATCH 10/21] Refactor parse method for improved readability and maintainability by extracting validation logic into separate methods --- lib/ruby_units/unit.rb | 114 +++++++++++++++++++---------------------- 1 file changed, 54 insertions(+), 60 deletions(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index be493a7..8d240e7 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -2040,7 +2040,7 @@ def unit_signature # @todo This should either be a separate class or at least a class method def parse(passed_unit_string = "0") unit_string = passed_unit_string.dup - unit_string = "#{Regexp.last_match(1)} USD" if unit_string =~ /\$\s*(#{NUMBER_REGEX})/ + unit_string = "#{Regexp.last_match('usd')} USD" if unit_string =~ /\$\s*(?#{NUMBER_REGEX})/ unit_string.gsub!("\u00b0".encode("utf-8"), "deg") if unit_string.encoding == Encoding::UTF_8 unit_string.gsub!(/(\d)[_,](\d)/, '\1\2') # remove underscores and commas in numbers @@ -2048,11 +2048,9 @@ def parse(passed_unit_string = "0") unit_string.gsub!(/[%'"#]/, "%" => "percent", "'" => "feet", '"' => "inch", "#" => "pound") if unit_string.start_with?(COMPLEX_NUMBER) match = unit_string.match(COMPLEX_REGEX) - real_str = match[:real] - imaginary_str = match[:imaginary] + real_str, imaginary_str, unit_s = match.values_at(:real, :imaginary, :unit) real = Float(real_str) if real_str imaginary = Float(imaginary_str) - unit_s = match[:unit] real_as_int = real.to_i if real real = real_as_int if real_as_int == real imaginary_as_int = imaginary.to_i @@ -2060,20 +2058,17 @@ def parse(passed_unit_string = "0") complex = Complex(real || 0, imaginary) complex_real = complex.real complex = complex.to_i if complex.imaginary.zero? && complex_real == complex_real.to_i - result = unit_class.new(unit_s || 1) * complex - copy(result) - return + return copy(unit_class.new(unit_s || 1) * complex) end if unit_string.start_with?(RATIONAL_NUMBER) match = unit_string.match(RATIONAL_REGEX) - numerator = Integer(match[:numerator]) - denominator = Integer(match[:denominator]) - proper_string = match[:proper] + numerator_string, denominator_string, proper_string, unit_s = match.values_at(:numerator, :denominator, :proper, :unit) + numerator = Integer(numerator_string) + denominator = Integer(denominator_string) raise ArgumentError, "Improper fractions must have a whole number part" if proper_string && !proper_string.match?(/^#{INTEGER_REGEX}$/) proper = proper_string.to_i - unit_s = match[:unit] fraction = Rational(numerator, denominator) rational = if proper.negative? (proper - fraction) @@ -2082,15 +2077,12 @@ def parse(passed_unit_string = "0") end rational_as_int = rational.to_int rational = rational_as_int if rational_as_int == rational - result = unit_class.new(unit_s || 1) * rational - copy(result) - return + return copy(unit_class.new(unit_s || 1) * rational) end match = unit_string.match(NUMBER_REGEX) - unit_str = match[:unit] + unit_str, scalar_str = match.values_at(:unit, :scalar) unit = unit_class.cached.get(unit_str) - scalar_str = match[:scalar] mult = scalar_str == "" ? 1.0 : scalar_str.to_f mult_as_int = mult.to_int mult = mult_as_int if mult_as_int == mult @@ -2113,63 +2105,50 @@ def parse(passed_unit_string = "0") unit_string.gsub!(/[<>]/, "") if (match = unit_string.match(TIME_REGEX)) - hours = match[:hour] - minutes = match[:min] - seconds = match[:sec] - milliseconds = match[:msec] + hours, minutes, seconds, milliseconds = match.values_at(:hour, :min, :sec, :msec) raise ArgumentError, "Invalid Duration" if [hours, minutes, seconds, milliseconds].all?(&:nil?) - result = unit_class.new("#{hours || 0} hours") + - unit_class.new("#{minutes || 0} minutes") + - unit_class.new("#{seconds || 0} seconds") + - unit_class.new("#{milliseconds || 0} milliseconds") - copy(result) - return + return copy(unit_class.new("#{hours || 0} hours") + + unit_class.new("#{minutes || 0} minutes") + + unit_class.new("#{seconds || 0} seconds") + + unit_class.new("#{milliseconds || 0} milliseconds")) end - # Special processing for unusual unit strings # feet -- 6'5" if (match = unit_string.match(FEET_INCH_REGEX)) - feet = Integer(match[:feet]) - inches = match[:inches] - result = if feet.negative? - unit_class.new("#{feet} ft") - unit_class.new("#{inches} inches") - else - unit_class.new("#{feet} ft") + unit_class.new("#{inches} inches") - end - copy(result) - return + feet_str, inches = match.values_at(:feet, :inches) + feet = Integer(feet_str) + return copy(if feet.negative? + unit_class.new("#{feet} ft") - unit_class.new("#{inches} inches") + else + unit_class.new("#{feet} ft") + unit_class.new("#{inches} inches") + end) end # weight -- 8 lbs 12 oz if (match = unit_string.match(LBS_OZ_REGEX)) - pounds = Integer(match[:pounds]) - oz = match[:oz] - result = if pounds.negative? - unit_class.new("#{pounds} lbs") - unit_class.new("#{oz} oz") - else - unit_class.new("#{pounds} lbs") + unit_class.new("#{oz} oz") - end - copy(result) - return + pounds_str, oz = match.values_at(:pounds, :oz) + pounds = Integer(pounds_str) + return copy(if pounds.negative? + unit_class.new("#{pounds} lbs") - unit_class.new("#{oz} oz") + else + unit_class.new("#{pounds} lbs") + unit_class.new("#{oz} oz") + end) end # stone -- 3 stone 5, 2 stone, 14 stone 3 pounds, etc. if (match = unit_string.match(STONE_LB_REGEX)) - stone = Integer(match[:stone]) - pounds = match[:pounds] - result = if stone.negative? - unit_class.new("#{stone} stone") - unit_class.new("#{pounds} lbs") - else - unit_class.new("#{stone} stone") + unit_class.new("#{pounds} lbs") - end - copy(result) - return + stone_str, pounds = match.values_at(:stone, :pounds) + stone = Integer(stone_str) + return copy(if stone.negative? + unit_class.new("#{stone} stone") - unit_class.new("#{pounds} lbs") + else + unit_class.new("#{stone} stone") + unit_class.new("#{pounds} lbs") + end) end # more than one per. I.e., "1 m/s/s" - raise(ArgumentError, "'#{passed_unit_string}' Unit not recognized") if unit_string.count("/") > 1 - raise(ArgumentError, "'#{passed_unit_string}' Unit not recognized #{unit_string}") if unit_string =~ /\s[02-9]/ + validate_unit_string_format(passed_unit_string, unit_string) @scalar, top, bottom = unit_string.scan(UNIT_STRING_REGEX)[0] # parse the string into parts top.scan(TOP_REGEX).each do |item| @@ -2212,13 +2191,11 @@ def parse(passed_unit_string = "0") # eliminate all known terms from this string. This is a quick check to see if the passed unit # contains terms that are not defined. used = "#{top} #{bottom}".gsub(unit_match_regex, "").gsub(%r{[\d*, "'_^/$]}, "") - raise(ArgumentError, "'#{passed_unit_string}' Unit not recognized") unless used.empty? + invalid_unit(passed_unit_string) unless used.empty? prefix_map = unit_class.prefix_map unit_map = unit_class.unit_map - transform_units = lambda do |item| - prefix = item[0] - unit = item[1] + transform_units = lambda do |(prefix, unit)| prefix_value = prefix_map[prefix] unit_value = unit_map[unit] prefix_value ? [prefix_value, unit_value] : [unit_value] @@ -2232,5 +2209,22 @@ def parse(passed_unit_string = "0") @denominator = UNITY_ARRAY if @denominator.empty? self end + + def validate_unit_string_format(passed_unit_string, unit_string) + slash_count = unit_string.count("/") + return if slash_count <= 1 && unit_string !~ /\s[02-9]/ + + if slash_count > 1 + invalid_unit(passed_unit_string) + else + invalid_unit(passed_unit_string, unit_string) + end + end + + def invalid_unit(unit_string, additional_info = nil) + error_msg = "'#{unit_string}' Unit not recognized" + error_msg += " #{additional_info}" if additional_info + raise ArgumentError, error_msg + end end end From b70585ca4e7f502249da48eafd533a3e9f7c6dec Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 09:40:25 -0500 Subject: [PATCH 11/21] Refactor unit methods for improved clarity and maintainability by reorganizing and enhancing documentation --- lib/ruby_units/unit.rb | 94 ++++++++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 44 deletions(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index 8d240e7..a077b25 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -416,7 +416,6 @@ def self.unit_match_regex # return a regexp fragment used to match prefixes # @return [String] - # @private def self.prefix_regex @prefix_regex ||= prefix_map.keys.sort_by { [_1.length, _1] }.reverse.join("|") end @@ -1553,22 +1552,6 @@ def hash # Protected and Private Functions that should only be called from this class protected - # Ensure that this unit is compatible with another unit - # @param [Object] other - # @return [void] - # @raise [ArgumentError] if units are not compatible - def ensure_compatible_with(other) - raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" unless compatible_with?(other) - end - - # Validate that a time_point is a Time, Date, or DateTime - # @param [Object] time_point - # @return [void] - # @raise [ArgumentError] when time_point is not a Time, Date, or DateTime - def validate_time_point(time_point) - raise ArgumentError, "Must specify a Time, Date, or DateTime" unless time_point.is_a?(Time) || time_point.is_a?(Date) || time_point.is_a?(DateTime) - end - # figure out what the scalar part of the base unit for this unit is # @return [nil] def update_base_scalar @@ -1620,12 +1603,28 @@ def apply_signature_items(vector, items, sign) end end + # Ensure that this unit is compatible with another unit + # @param [Object] other the unit to check compatibility with + # @return [void] + # @raise [ArgumentError] if units are not compatible + def ensure_compatible_with(other) + raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" unless compatible_with?(other) + end + + # Validate that a time_point is a Time, Date, or DateTime + # @param [Object] time_point the object to validate + # @return [void] + # @raise [ArgumentError] when time_point is not a Time, Date, or DateTime + def validate_time_point(time_point) + raise ArgumentError, "Must specify a Time, Date, or DateTime" unless time_point.is_a?(Time) || time_point.is_a?(Date) || time_point.is_a?(DateTime) + end + # Helper methods for power operation # Validate that power operation is allowed - # @param exponent [Numeric] the exponent + # @param exponent [Numeric] the exponent value # @return [void] - # @raise [ArgumentError] if operation is not allowed + # @raise [ArgumentError] if operation is not allowed (temperature units or non-integer exponent) def validate_power_operation(exponent) raise ArgumentError, "Cannot raise a temperature to a power" if temperature? raise ArgumentError, "Exponent must an Integer" unless exponent.is_a?(Integer) @@ -1714,11 +1713,13 @@ def convert_string_target(target_units, format) end # Convert with a format string - # @param format_str [String] the format string - # @param target_unit [String, nil] the target unit - # @param original_target [String] the original target_units string (for strftime fallback) - # @param format [Symbol] the format to use - # @return [String] formatted string + # Handles formatting of unit conversions with custom format strings or strftime patterns + # @param format_str [String] the format string (e.g., '%0.2f') + # @param target_unit [String, nil] the target unit to convert to, nil for no conversion + # @param original_target [String] the original target_units string for strftime fallback + # @param format [Symbol] the output format symbol + # @return [String] the formatted unit string + # @raise [StandardError] caught and handled by attempting strftime parsing def convert_with_format_string(format_str, target_unit, original_target, format) if target_unit # unit specified, need to convert convert_to(target_unit).to_s(format_str, format: format) @@ -1730,9 +1731,10 @@ def convert_with_format_string(format_str, target_unit, original_target, format) (DateTime.new(0) + self).strftime(original_target) end - # Format the scalar value - # @param unit_str [String] the unit string - # @return [String] formatted string + # Format the scalar value with appropriate separator and unit string + # Handles Complex, Rational, and numeric scalars appropriately + # @param unit_str [String] the unit string to append + # @return [String] the formatted scalar with separator and unit string def format_scalar(unit_str) separator = RubyUnits.configuration.separator is_integer = scalar_is_integer? @@ -1746,45 +1748,49 @@ def format_scalar(unit_str) end.strip end - # Helper to check if scalar is effectively an integer - # @return [Boolean] + # Check if the scalar is effectively an integer + # Handles Complex numbers by returning false, and compares the scalar to its integer representation + # @return [Boolean] true if scalar equals its integer representation, false otherwise def scalar_is_integer? return false if @scalar.is_a?(Complex) @scalar == @scalar.to_i end - # Helper to create a new unit with modified scalar but same units - # @param [Numeric] new_scalar - # @return [Unit] + # Create a new unit with a modified scalar but the same units + # @param new_scalar [Numeric] the new scalar value for the unit + # @return [Unit] a new unit with the same numerator and denominator but different scalar def with_new_scalar(new_scalar) unit_class.new(scalar: new_scalar, numerator: @numerator, denominator: @denominator) end - # used by #dup to duplicate a Unit - # @param [Unit] other - # @private + # Initialize copy: used by #dup to duplicate a Unit + # Duplicates the numerator and denominator arrays to ensure deep copying + # @param other [Unit] the unit to copy from def initialize_copy(other) @numerator = other.numerator.dup @denominator = other.denominator.dup + super end - # Return scalar if unitless, otherwise raise an error - # @param method [Symbol] method to call on scalar - # @param type [Class] the type being converted to (for error message) - # @param converter [Proc] optional converter proc (defaults to calling method on scalar) - # @return [Numeric] - # @raise [RuntimeError] when not unitless + # Return the scalar if unitless, otherwise raise an error + # This helper method is used by conversion methods like #to_f, #to_i, #to_c, #to_r + # @param method [Symbol] the method to call on the scalar (e.g., :to_f, :to_i) + # @param type [Class] the type being converted to (used in error message) + # @param converter [Proc, nil] optional converter proc; if provided, called instead of method + # @return [Numeric] the scalar converted using the provided method or converter + # @raise [RuntimeError] when the unit is not unitless def return_scalar_or_raise(method, type, converter = nil) raise "Cannot convert '#{self}' to #{type} unless unitless. Use Unit#scalar" unless unitless? converter ? converter.call(@scalar) : @scalar.public_send(method) end - # Return scalar if unitless, otherwise return a new unit with the modified scalar + # Return the scalar if unitless, otherwise return a new unit with the modified scalar + # This helper method is used by unary operations like #-@, #abs, #ceil, #floor, #round, #truncate # @param scalar_value [Numeric] the scalar value to return if unitless - # @param new_scalar [Numeric] the new scalar for the unit if not unitless - # @return [Numeric,Unit] + # @param new_scalar [Numeric] the new scalar value for the unit if not unitless + # @return [Numeric, Unit] the scalar if unitless, or a new unit with the modified scalar def return_scalar_or_unit(scalar_value, new_scalar) return scalar_value if unitless? From e861f459594e8f329eb65f662ad99264b3464e6b Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 10:51:48 -0500 Subject: [PATCH 12/21] Refactor return_scalar_or_raise method to simplify conversion logic and improve readability --- lib/ruby_units/unit.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index a077b25..e1ccbd5 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -1246,7 +1246,7 @@ def to_f # @return [Complex] # @raise [RuntimeError] when not unitless def to_c - return_scalar_or_raise(:to_c, Complex, ->(s) { Complex(s) }) + return_scalar_or_raise(:to_c, Complex) end # if unitless, returns an int, otherwise raises an error @@ -1777,13 +1777,12 @@ def initialize_copy(other) # This helper method is used by conversion methods like #to_f, #to_i, #to_c, #to_r # @param method [Symbol] the method to call on the scalar (e.g., :to_f, :to_i) # @param type [Class] the type being converted to (used in error message) - # @param converter [Proc, nil] optional converter proc; if provided, called instead of method - # @return [Numeric] the scalar converted using the provided method or converter + # @return [Numeric] the scalar converted using the provided method # @raise [RuntimeError] when the unit is not unitless - def return_scalar_or_raise(method, type, converter = nil) + def return_scalar_or_raise(method, type) raise "Cannot convert '#{self}' to #{type} unless unitless. Use Unit#scalar" unless unitless? - converter ? converter.call(@scalar) : @scalar.public_send(method) + @scalar.public_send(method) end # Return the scalar if unitless, otherwise return a new unit with the modified scalar From 041a8b491a7b8bb25053207723e54a3b8bc7d0e4 Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 10:55:25 -0500 Subject: [PATCH 13/21] Refactor return_scalar_or_unit method to simplify parameter usage and improve clarity in unary operations --- lib/ruby_units/unit.rb | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index e1ccbd5..0a0cc24 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -1321,14 +1321,14 @@ def units(with_prefix: true, format: nil) # @return [Numeric,Unit] def -@ neg_scalar = -@scalar - return_scalar_or_unit(neg_scalar, neg_scalar) + return_scalar_or_unit(neg_scalar) end # absolute value of a unit # @return [Numeric,Unit] def abs abs_scalar = @scalar.abs - return_scalar_or_unit(abs_scalar, abs_scalar) + return_scalar_or_unit(abs_scalar) end # ceil of a unit @@ -1336,7 +1336,7 @@ def abs # @return [Numeric,Unit] def ceil(...) ceiled_scalar = @scalar.ceil(...) - return_scalar_or_unit(ceiled_scalar, ceiled_scalar) + return_scalar_or_unit(ceiled_scalar) end # Floor of a unit @@ -1344,7 +1344,7 @@ def ceil(...) # @return [Numeric,Unit] def floor(...) floored_scalar = @scalar.floor(...) - return_scalar_or_unit(floored_scalar, floored_scalar) + return_scalar_or_unit(floored_scalar) end # Round the unit according to the rules of the scalar's class. Call this @@ -1360,7 +1360,7 @@ def floor(...) # @return [Numeric,Unit] def round(...) rounded_scalar = @scalar.round(...) - return_scalar_or_unit(rounded_scalar, rounded_scalar) + return_scalar_or_unit(rounded_scalar) end # Truncate the unit according to the scalar's truncate method @@ -1368,7 +1368,7 @@ def round(...) # @return [Numeric, Unit] def truncate(...) truncated_scalar = @scalar.truncate(...) - return_scalar_or_unit(truncated_scalar, truncated_scalar) + return_scalar_or_unit(truncated_scalar) end # Returns next unit in a range. Increments the scalar by 1. @@ -1787,11 +1787,10 @@ def return_scalar_or_raise(method, type) # Return the scalar if unitless, otherwise return a new unit with the modified scalar # This helper method is used by unary operations like #-@, #abs, #ceil, #floor, #round, #truncate - # @param scalar_value [Numeric] the scalar value to return if unitless - # @param new_scalar [Numeric] the new scalar value for the unit if not unitless + # @param new_scalar [Numeric] the new scalar value # @return [Numeric, Unit] the scalar if unitless, or a new unit with the modified scalar - def return_scalar_or_unit(scalar_value, new_scalar) - return scalar_value if unitless? + def return_scalar_or_unit(new_scalar) + return new_scalar if unitless? with_new_scalar(new_scalar) end From 1a9b19c4bef44ddc1baacf2fc3651c7f2deed3cc Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 11:14:49 -0500 Subject: [PATCH 14/21] Refactor error messages for consistency in power operation validation --- .reek.yml | 12 +++++++----- lib/ruby_units/unit.rb | 2 +- spec/ruby_units/unit_spec.rb | 6 +++--- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/.reek.yml b/.reek.yml index 9dca8a6..e525e7e 100644 --- a/.reek.yml +++ b/.reek.yml @@ -1,6 +1,6 @@ # Reek configuration # -# InstanceVariableAssumption is disabled because the Unit class uses a mix of +# InstanceVariableAssumption is disabled for Unit class because it uses a mix of # eager initialization (in initialize_instance_variables) and lazy initialization # (using defined? checks) for its instance variables. This is an intentional design # pattern where some variables are initialized immediately and others are computed @@ -8,8 +8,8 @@ # # See: https://github.com/troessner/reek/blob/master/docs/Instance-Variable-Assumption.md # -# RepeatedConditional warnings are configured below to exclude legitimate cases where -# the same condition needs to be checked in different methods for different purposes: +# RepeatedConditional warnings are excluded for Unit class where the same condition +# needs to be checked in different methods for different purposes: # # - base?: Checked in to_base (early return), update_base_scalar (base unit calculation), # and unit_signature_vector (recursion avoidance). Each serves a distinct purpose. @@ -26,7 +26,9 @@ detectors: InstanceVariableAssumption: - enabled: false + exclude: + - RubyUnits::Unit RepeatedConditional: - enabled: false + exclude: + - RubyUnits::Unit diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index 0a0cc24..4e9f427 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -1627,7 +1627,7 @@ def validate_time_point(time_point) # @raise [ArgumentError] if operation is not allowed (temperature units or non-integer exponent) def validate_power_operation(exponent) raise ArgumentError, "Cannot raise a temperature to a power" if temperature? - raise ArgumentError, "Exponent must an Integer" unless exponent.is_a?(Integer) + raise ArgumentError, "Exponent must be an Integer" unless exponent.is_a?(Integer) end # Handle special cases for power operation diff --git a/spec/ruby_units/unit_spec.rb b/spec/ruby_units/unit_spec.rb index 5970f3d..1e3a294 100644 --- a/spec/ruby_units/unit_spec.rb +++ b/spec/ruby_units/unit_spec.rb @@ -2208,15 +2208,15 @@ subject { RubyUnits::Unit.new("1 m") } it "raises an exception when passed a Float argument" do - expect { subject.power(1.5) }.to raise_error(ArgumentError, "Exponent must an Integer") + expect { subject.power(1.5) }.to raise_error(ArgumentError, "Exponent must be an Integer") end it "raises an exception when passed a Rational argument" do - expect { subject.power(Rational(1, 2)) }.to raise_error(ArgumentError, "Exponent must an Integer") + expect { subject.power(Rational(1, 2)) }.to raise_error(ArgumentError, "Exponent must be an Integer") end it "raises an exception when passed a Complex argument" do - expect { subject.power(Complex(1, 2)) }.to raise_error(ArgumentError, "Exponent must an Integer") + expect { subject.power(Complex(1, 2)) }.to raise_error(ArgumentError, "Exponent must be an Integer") end it "raises an exception when called on a temperature unit" do From 95fa13808e4a7bd02ece89d53eca1c7c3cb38c1c Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 11:29:48 -0500 Subject: [PATCH 15/21] Refactor simplify_rational method to improve clarity and encapsulate functionality within the class --- lib/ruby_units/unit.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index 4e9f427..52eb339 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -47,6 +47,13 @@ class << self private attr_writer :definitions, :prefix_values, :prefix_map, :unit_map, :unit_values + + # Helper to simplify a rational by returning numerator if denominator is 1 + # @param [Rational] rational + # @return [Integer, Rational] + def simplify_rational(rational) + rational.denominator == 1 ? rational.numerator : rational + end end self.definitions = {} self.prefix_values = {} @@ -1581,13 +1588,6 @@ def unit_signature_vector vector end - # Helper to simplify a rational by returning numerator if denominator is 1 - # @param [Rational] rational - # @return [Integer, Rational] - def self.simplify_rational(rational) - rational.denominator == 1 ? rational.numerator : rational - end - private # Internal helper for unit_signature_vector From 4793e58ca585066c2ec54d7c955329c886c64bf9 Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 11:52:11 -0500 Subject: [PATCH 16/21] Refactor simplify_rational method to be a class method and improve visibility; reorganize private methods for better structure --- lib/ruby_units/unit.rb | 45 +++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index 52eb339..bbfacc0 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -47,13 +47,6 @@ class << self private attr_writer :definitions, :prefix_values, :prefix_map, :unit_map, :unit_values - - # Helper to simplify a rational by returning numerator if denominator is 1 - # @param [Rational] rational - # @return [Integer, Rational] - def simplify_rational(rational) - rational.denominator == 1 ? rational.numerator : rational - end end self.definitions = {} self.prefix_values = {} @@ -525,6 +518,14 @@ def self.format_fraction(frac, precision: RubyUnits.configuration.default_precis "-#{rationalized}" end + # Helper to simplify a rational by returning numerator if denominator is 1 + # @param [Rational] rational + # @return [Integer, Rational] + # @private + def self.simplify_rational(rational) + rational.denominator == 1 ? rational.numerator : rational + end + include Comparable # @return [Numeric] @@ -1559,19 +1560,6 @@ def hash # Protected and Private Functions that should only be called from this class protected - # figure out what the scalar part of the base unit for this unit is - # @return [nil] - def update_base_scalar - if base? - @base_scalar = @scalar - @signature = unit_signature - else - base = to_base - @base_scalar = base.scalar - @signature = base.signature - end - end - # calculates the unit signature vector used by unit_signature # @return [Array] # @raise [ArgumentError] when exponent associated with a unit is > 20 or < -20 @@ -1588,8 +1576,6 @@ def unit_signature_vector vector end - private - # Internal helper for unit_signature_vector # Applies unit definitions from items to the signature vector with a sign # @param vector [Array] signature accumulation array @@ -1603,6 +1589,21 @@ def apply_signature_items(vector, items, sign) end end + private + + # figure out what the scalar part of the base unit for this unit is + # @return [nil] + def update_base_scalar + if base? + @base_scalar = @scalar + @signature = unit_signature + else + base = to_base + @base_scalar = base.scalar + @signature = base.signature + end + end + # Ensure that this unit is compatible with another unit # @param [Object] other the unit to check compatibility with # @return [void] From 1e11a300cf385533c14f6c353a76c321058dbe7c Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 12:39:15 -0500 Subject: [PATCH 17/21] Refactor regex patterns in unit parsing for improved clarity and maintainability --- lib/ruby_units/unit.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index bbfacc0..89ec551 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -96,8 +96,8 @@ class << self ANY_NUMBER_REGEX = /(?:#{ANY_NUMBER})?\s?([^-\d.].*)?/ NUMBER_REGEX = /(?#{SCI_NUMBER}*)\s*(?.+)?/ # a number followed by a unit UNIT_STRING_REGEX = %r{#{SCI_NUMBER}*\s*([^/]*)/*(.+)*} - TOP_REGEX = /([^ *]+)(?:\^|\*\*)([\d-]+)/ - BOTTOM_REGEX = /([^* ]+)(?:\^|\*\*)(\d+)/ + TOP_REGEX = /(?[^ *]+)(?:\^|\*\*)(?[\d-]+)/ + BOTTOM_REGEX = /(?[^* ]+)(?:\^|\*\*)(?\d+)/ NUMBER_UNIT_REGEX = /#{SCI_NUMBER}?(.*)/ COMPLEX_REGEX = /#{COMPLEX_NUMBER}\s?(?.+)?/ RATIONAL_REGEX = /#{RATIONAL_NUMBER}\s?(?.+)?/ @@ -2156,9 +2156,8 @@ def parse(passed_unit_string = "0") validate_unit_string_format(passed_unit_string, unit_string) @scalar, top, bottom = unit_string.scan(UNIT_STRING_REGEX)[0] # parse the string into parts - top.scan(TOP_REGEX).each do |item| - unit_part = item[0] - exponent = item[1].to_i + top.scan(TOP_REGEX).each do |(unit_part, exponent_string)| + exponent = exponent_string.to_i unit_with_space = "#{unit_part} " if exponent >= 0 top.gsub!(/#{unit_part}(\^|\*\*)#{exponent}/) { unit_with_space * exponent } @@ -2168,7 +2167,10 @@ def parse(passed_unit_string = "0") end end if bottom - bottom.gsub!(BOTTOM_REGEX) { "#{Regexp.last_match(1)} " * Regexp.last_match(2).to_i } + bottom.gsub!(BOTTOM_REGEX) do + unit_part, bottom_exponent_string = Regexp.last_match.values_at(:unit_part, :exponent) + "#{unit_part} " * bottom_exponent_string.to_i + end # Separate leading decimal from denominator, if any bottom_scalar, bottom = bottom.scan(NUMBER_UNIT_REGEX)[0] end From c18cc6c304f79753d679d086f517c9b1ed1e6f46 Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 13:03:17 -0500 Subject: [PATCH 18/21] Refactor error messages for unit compatibility to include unit values for clarity --- lib/ruby_units/unit.rb | 20 +++++++++----------- spec/ruby_units/numeric_spec.rb | 2 +- spec/ruby_units/time_spec.rb | 2 +- spec/ruby_units/unit_spec.rb | 6 +++--- 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index 89ec551..48f06c4 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -794,8 +794,7 @@ def <=>(other) elsif !temperature? && other.respond_to?(:zero?) && other.zero? base_scalar <=> 0 elsif other.instance_of?(Unit) - raise ArgumentError, "Incompatible Units ('#{units}' not compatible with '#{other.units}')" unless self =~ other - + ensure_compatible_with(other) base_scalar <=> other.base_scalar else coerced_unit, coerced_other = coerce(other) @@ -815,7 +814,7 @@ def ==(other) if other.respond_to?(:zero?) && other.zero? zero? elsif other.instance_of?(Unit) - return false unless self =~ other + return false unless compatible_with?(other) base_scalar == other.base_scalar else @@ -886,9 +885,7 @@ def +(other) when Unit if zero? other.dup - else - compatible = self =~ other - raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" unless compatible + elsif compatible_with?(other) raise ArgumentError, "Cannot add two temperatures" if [self, other].all?(&:temperature?) if temperature? @@ -898,6 +895,8 @@ def +(other) else unit_class.new(scalar: (base_scalar + other.base_scalar), numerator: base.numerator, denominator: base.denominator, signature: @signature).convert_to(self) end + else + raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" end when Date, Time raise ArgumentError, "Date and Time objects represent fixed points in time and cannot be added to a Unit" @@ -923,10 +922,7 @@ def -(other) else -other_copy end - else - compatible = self =~ other - raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" unless compatible - + elsif compatible_with?(other) scalar_difference = base_scalar - other.base_scalar if [self, other].all?(&:temperature?) unit_class.new(scalar: scalar_difference, numerator: KELVIN, denominator: UNITY_ARRAY, signature: @signature).convert_to(temperature_scale) @@ -937,6 +933,8 @@ def -(other) else unit_class.new(scalar: scalar_difference, numerator: base.numerator, denominator: base.denominator, signature: @signature).convert_to(self) end + else + raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" end when Time raise ArgumentError, "Date and Time objects represent fixed points in time and cannot be subtracted from a Unit" @@ -1214,7 +1212,7 @@ def convert_to(other) end return self if target.units == units - raise ArgumentError, "Incompatible Units ('#{self}' not compatible with '#{other}')" unless self =~ target + ensure_compatible_with(target) prefix_vals = unit_class.prefix_values unit_vals = unit_class.unit_values diff --git a/spec/ruby_units/numeric_spec.rb b/spec/ruby_units/numeric_spec.rb index 4f76ecb..ae0bd46 100644 --- a/spec/ruby_units/numeric_spec.rb +++ b/spec/ruby_units/numeric_spec.rb @@ -26,7 +26,7 @@ end it "raises an exception if the unit is not unitless" do - expect { 0.1.to_unit("m") }.to raise_error(ArgumentError, "Incompatible Units ('0.1' not compatible with 'm')") + expect { 0.1.to_unit("m") }.to raise_error(ArgumentError, "Incompatible Units ('0.1' not compatible with '1 m')") expect { 0.1.to_unit(RubyUnits::Unit.new("1 m")) }.to raise_error(ArgumentError, "Incompatible Units ('0.1' not compatible with '1 m')") end end diff --git a/spec/ruby_units/time_spec.rb b/spec/ruby_units/time_spec.rb index 118e200..9ca6cd5 100644 --- a/spec/ruby_units/time_spec.rb +++ b/spec/ruby_units/time_spec.rb @@ -47,7 +47,7 @@ describe ".in" do specify { expect(Time.in("5 min")).to have_attributes(to_s: "2011-04-24 10:51:30 -0400") } specify { expect(Time.in([5, "min"])).to have_attributes(to_s: "2011-04-24 10:51:30 -0400") } - specify { expect { Time.in(300) }.to raise_error(ArgumentError, "Incompatible Units ('300' not compatible with 's')") } + specify { expect { Time.in(300) }.to raise_error(ArgumentError, "Incompatible Units ('300' not compatible with '1 s')") } end describe "#to_unit" do diff --git a/spec/ruby_units/unit_spec.rb b/spec/ruby_units/unit_spec.rb index 1e3a294..8702996 100644 --- a/spec/ruby_units/unit_spec.rb +++ b/spec/ruby_units/unit_spec.rb @@ -1839,8 +1839,8 @@ end context "incompatible units cannot be compared" do - specify { expect { RubyUnits::Unit.new("1 m") < RubyUnits::Unit.new("1 liter") }.to raise_error(ArgumentError, "Incompatible Units ('m' not compatible with 'l')") } - specify { expect { RubyUnits::Unit.new("1 kg") > RubyUnits::Unit.new("60 mph") }.to raise_error(ArgumentError, "Incompatible Units ('kg' not compatible with 'mph')") } + specify { expect { RubyUnits::Unit.new("1 m") < RubyUnits::Unit.new("1 liter") }.to raise_error(ArgumentError, "Incompatible Units ('1 m' not compatible with '1 l')") } + specify { expect { RubyUnits::Unit.new("1 kg") > RubyUnits::Unit.new("60 mph") }.to raise_error(ArgumentError, "Incompatible Units ('1 kg' not compatible with '60 mph')") } end context "with coercions should be valid" do @@ -1860,7 +1860,7 @@ end context "between incompatible units" do - specify { expect { RubyUnits::Unit.new("1 s").convert_to("m") }.to raise_error(ArgumentError, "Incompatible Units ('1 s' not compatible with 'm')") } + specify { expect { RubyUnits::Unit.new("1 s").convert_to("m") }.to raise_error(ArgumentError, "Incompatible Units ('1 s' not compatible with '1 m')") } end context "given bad input" do From 9c95df5bc3439d39bc5cc1bd7691faf930ffaa38 Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 17:12:52 -0500 Subject: [PATCH 19/21] Enhance documentation for unit methods with parameter and return type annotations --- lib/ruby_units/unit.rb | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index 48f06c4..eab6cf5 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -462,6 +462,8 @@ def self.use_definition(definition) end # Invalidate regex cache for unit parsing + # + # @return [void] def self.invalidate_regex_cache @unit_match_regex = nil @temp_regex = nil @@ -469,7 +471,9 @@ def self.invalidate_regex_cache end # Register a prefix definition + # # @param definition [RubyUnits::Unit::Definition] + # @return [void] def self.register_prefix_definition(definition) definition_name = definition.name prefix_values[definition_name] = definition.scalar @@ -478,7 +482,9 @@ def self.register_prefix_definition(definition) end # Register a unit definition + # # @param definition [RubyUnits::Unit::Definition] + # @return [void] def self.register_unit_definition(definition) definition_name = definition.name unit_value = create_unit_value(definition) @@ -500,9 +506,11 @@ def self.create_unit_value(definition) end # Register aliases for a definition - # @param aliases [Array] the aliases to register + # + # @param aliases [Array] the aliases to register # @param name [String] the canonical name # @param map [Hash] the map to register aliases in + # @return [void] def self.register_aliases(aliases, name, map) aliases.each { map[_1] = name } end @@ -1543,6 +1551,9 @@ def best_prefix end # override hash method so objects with same values are considered equal + # override hash method so objects with same values are considered equal + # + # @return [Integer] def hash [ @scalar, @@ -2215,6 +2226,14 @@ def parse(passed_unit_string = "0") self end + # Validate the basic format of a parsed unit string. + # Ensures there is at most one '/' and that there are no stray digits + # in the unit portion (which indicate malformed input). + # + # @param passed_unit_string [String] the original string passed by the caller + # @param unit_string [String] the normalized unit portion being validated + # @return [void] + # @raise [ArgumentError] when the unit string is malformed def validate_unit_string_format(passed_unit_string, unit_string) slash_count = unit_string.count("/") return if slash_count <= 1 && unit_string !~ /\s[02-9]/ @@ -2226,6 +2245,11 @@ def validate_unit_string_format(passed_unit_string, unit_string) end end + # Raise a standardized ArgumentError for an unrecognized unit string. + # + # @param unit_string [String] the (possibly invalid) unit text + # @param additional_info [String, nil] optional additional context to include + # @raise [ArgumentError] def invalid_unit(unit_string, additional_info = nil) error_msg = "'#{unit_string}' Unit not recognized" error_msg += " #{additional_info}" if additional_info From c01e0c0c689cc33276278afe5dbf6d63727279e0 Mon Sep 17 00:00:00 2001 From: Kevin Olbrich Date: Sun, 28 Dec 2025 18:09:05 -0500 Subject: [PATCH 20/21] Remove redundant comment in hash method override for clarity --- lib/ruby_units/unit.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/ruby_units/unit.rb b/lib/ruby_units/unit.rb index eab6cf5..db472e2 100644 --- a/lib/ruby_units/unit.rb +++ b/lib/ruby_units/unit.rb @@ -1550,7 +1550,6 @@ def best_prefix to(unit_class.new(unit_class.prefix_map.key(best_prefix) + units(with_prefix: false))) end - # override hash method so objects with same values are considered equal # override hash method so objects with same values are considered equal # # @return [Integer] From dd84eee6a359b5be4b3553987b51df696db90553 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 28 Dec 2025 23:11:45 +0000 Subject: [PATCH 21/21] Initial plan