4
\$\begingroup\$

I've written this class for two purposes (I think only this fact shows that this code violates SRP): convert weight from one unit to another and to represent them accurate or not accurate. So class methods can be used for calculation and the present method rounds the value for presentation. I don't like this code - can you propose better solution?

class Temperature

  UNITS = %w(C F)
  DEFAULT_UNITS = "C"

  attr_reader :value, :units

  def initialize(value, units = DEFAULT_UNITS)
    @value = value
    @units = units.to_s
  end

  def as(units, accurate)
    return unless UNITS.include?(units.to_s)
    value = units.to_s == @units ? @value : self.class.try("#{@units.downcase}_to_#{units.downcase}", @value)
    accurate ? value : self.class.new(value, units).present
  end

  def default(accurate = false)
    as(DEFAULT_UNITS, accurate)
  end

  def present
    value.round(1)
  end

  def self.c_to_f(val)
    val * 9 / 5 + 32
  end

  def self.f_to_c(val)
    (val - 32) * 5 / 9
  end

end
\$\endgroup\$
3
\$\begingroup\$
class Numeric

  def fahrenheit_to_celsius
    (self - 32) * 5 / 9
  end

  def celsius_to_fahrenheit
    self * 9 / 5 + 32
  end

end

p -7.5.celsius_to_fahrenheit #=> 18.5

Classes are open for a reason in ruby. A temperature is just a number, converting it is just a method. The accuracy thing disappears by itself - an integer results in an integer, a float will give a float.

\$\endgroup\$
2
\$\begingroup\$

I would separated logic for Celsius and Fahrenheit into separate classes. Here is a code to demonstrate the general approach:

class Temperature
  attr_reader :value

  def initialize value # or you can go further and dont
    @value = value
  end
end

class Celsius < Temperature    
  def to_fahrenheit
    Fahrenheit.new value * 9 / 5 + 32
  end
end

class Fahrenheit < Temperature
  def to_celsius
    Celsius.new (value - 32) * 5 / 9
  end
end

With this approach you have good abstractions for different types of temperature and you don't mess different logic in the single class. Probably you will not need 'unit' property because you will have class names holding it (you can add it easily in both classes if you want).

Then you can write factory methods in Numeric class to write more clear code with temperature classes:

class Numeric
  def fahrenheit
    Fahrenheit.new(self)
  end

  def celsius
    Celsius.new(self)
  end
end

cels = -5.celsius
cels.to_fahrenheit.value # => 23

Regarding accurate property - you can accept it in the initializer or you can accept it in conversion methods (to_celsius, to_fahrenheit).


UPDATE

You can also implement accurate as a mixin (kind of decorator):

module Inaccurate
  def value
    @value.round(1)
  end
end

class Numeric
  def fahrenheit accurate = true
    Fahrenheit.new(self).tap { |t| t.extend(Inaccurate) unless accurate }
  end

  def celsius accurate = true
    Celsius.new(self).tap { |t| t.extend(Inaccurate) unless accurate }
  end
end

puts -5.45.celsius.value         # => -5.45
puts -5.45.celsius(false).value  # => -5.5

UPDATE

Temperature is abstraction of real-world temperature. Your as conversion method is specific to your application's data format ('C' and 'F' strings in user profile). Taking SRP principle into account, it is better to extract this conversion logic from Temperature class. You can do this with module, something like this:

module UserTemperatureConverter
  def as value, t_type, accurate = true
      case t_type
    when 'C'
      value.celsius(accurate)
    when 'F'
      value.fahrenheit(accurate)
    end
  end

  extend self # make 'as' a class method of UserTemperatureConverter
end
Temperature.extend UserTemperatureConverter # make 'as' a class method of Temperature

UserTemperatureConverter.as(17, 'C') # => Celsius
# or
Temperature.as(18, 'F') # => Celsius

# or make temperature conversion instance method and include it into `Temperature` as `Temperature#as`

Also you may find useful in your application to define basic self-conversion methods for Celsius and Fahrenteit:

class Celsius
  def to_celsius; self; end
end

class Fahrenheit
  def to_fahrenheit; self; end
end

17.celsius.to_celsius # Celsius
\$\endgroup\$
  • \$\begingroup\$ Cool approach! But I need method as() instead of named methods because I get units from the current_user profile to display values in desired units. \$\endgroup\$ – Antiarchitect Feb 6 '12 at 9:16
  • \$\begingroup\$ @Antiarchitect, added some thoughts on how you may implement temperature conversion/construction logic with my approach. \$\endgroup\$ – Aliaksei Kliuchnikau Feb 6 '12 at 11:55
0
\$\begingroup\$

First of all, don't use plural identifiers if the variable or constant doesn't hold multiple values (e.g. rename DEFAULT_UNITS to DEFAULT_UNIT and units to unit) as it can be very confusing.


Why don't you just always store the temperature in ℃? Having only one possible data representation in a class can significantly simplify the methods in that class. Convert lazily to ℉ lazily, when you need to.

class Temperature
  ...

  def initialize(value, unit = DEFAULT_UNIT)
    raise SomeError unless UNITS.include?(unit)
    if unit == "C"
      @value = value
    else
      @value = (value - 32) * 5 / 9
    end
  end

  ...
end

Also, in your Temperature#as method you might want to change the ternary operator to an if-else statement to make it more readable.

\$\endgroup\$
  • \$\begingroup\$ Measurements::Temperature stores the temperature values only in ℃. I cannot predict in what format temperature will be created: it has a value field and units accessor: \$\endgroup\$ – Antiarchitect Feb 1 '12 at 9:01
0
\$\begingroup\$

I would avoid to maintain different classes for different temperature scales. I mae an example based on Kelvin.

With Temperature.new( value[, unit]) you can define a temperature, the unit is optional.

Later you have access to the values with Temperature#celcius, Temperature#kelvin od Temperature#fahrenheit. If you need other scales (e.g. Réaumur) you could add them without any problem.

The as is also implemented, it has two parameters: The unit and a rounding factor.

My code:

class Temperature

  class Too_Cold < ArgumentError; end
  UNITS = %w(C F K)
  DEFAULT_UNIT = "K"

  attr_reader :kelvin

  def initialize(value, unit = DEFAULT_UNIT)
    case unit
      when 'K'
        @kelvin = value
      when 'C'         
        @kelvin       = 273.15 + value
      when 'F'
        @kelvin = (value + 459.67) * 5/9
      else
        raise ArgumentError, "Unknown Unit #{unit}"
      end
      raise Too_Cold if @kelvin < 0

      @celcius    =   @kelvin - 273.15
      @fahrenheit = @kelvin * 1.8 - 459.67
  end

  attr_reader :kelvin
  attr_reader :celcius
  attr_reader :fahrenheit

  def ==(other)
    @kelvin == other.kelvin
  end

  def as(unit, round = nil)
    return unless UNITS.include?(unit.to_s)
    value = case unit
      when 'C'; @celcius
      when 'F'; @fahrenheit
      when 'K'; @kelvin
    end
    round ? value.round(round) : value
  end

  def default(accurate = false)
    as(DEFAULT_UNITS, accurate)
  end

  def to_s(unit=DEFAULT_UNIT)
    "#{as(unit,2)}#{unit}"
  end
end

I calculated once the values for all units. You could also re-implement the class and calculate the value only if you need it.

And a corresponding test code. Please see the assert_in_delta-checks. I used float operations, so test on equality may fail.

require 'test/unit'

class TempTest < Test::Unit::TestCase
  def test_unit
    assert_raise(ArgumentError){ Temperature.new(0,'X')}
    assert_nothing_raised{ Temperature.new(0,'K')}
    assert_nothing_raised{ Temperature.new(0,'C')}
    assert_nothing_raised{ Temperature.new(0,'F')}
  end
  def test_0K
    assert_nothing_raised{ Temperature.new(0,'K') }
    assert_nothing_raised{ Temperature.new(-273.15,'C')}
    assert_nothing_raised{ Temperature.new(-459.67,'F')}
    assert_equal(0, Temperature.new(0,'K').kelvin)
    assert_equal(-273.15, Temperature.new(0,'K').celcius)
    assert_equal(-459.67, Temperature.new(0,'K').fahrenheit)
    assert_equal(Temperature.new(0,'K'), Temperature.new(-273.15,'C'))
    assert_equal(Temperature.new(0,'K'), Temperature.new(-459.67,'F'))
  end
  def test_too_cold
    assert_raise(Temperature::Too_Cold){ Temperature.new(-1,'K') }
    assert_raise(Temperature::Too_Cold){ Temperature.new(-273.16,'C')}
    assert_raise(Temperature::Too_Cold){ Temperature.new(-459.68,'F')}
  end
  def test_similar
    -200.upto(300){|i|
        assert_in_delta(i, Temperature.new(i,'K').kelvin) if i >= 0
        assert_in_delta(i, Temperature.new(i,'C').celcius)
        assert_in_delta(i, Temperature.new(i,'F').fahrenheit)
      }
  end

  def test_as_kelvin
    assert_equal(0, Temperature.new(0,'K').as('K'))
  end
  def test_as_celcius
    assert_in_delta(10, Temperature.new(10,'C').as('C'))
    assert_in_delta(10.2, Temperature.new(10.2,'C').as('C'))
    assert_in_delta(10, Temperature.new(10.2,'C').as('C',0))

    assert_equal(10, Temperature.new(10.2,'C').as('C',0))
    assert_equal(10.2, Temperature.new(10.2,'C').as('C',2))
  end
  def test_as_fahrenheit
    assert_in_delta(10, Temperature.new(10,'F').as('F'))
    assert_in_delta(10.2, Temperature.new(10.2,'F').as('F'))
    assert_in_delta(10, Temperature.new(10.2,'F').as('F',0))

    assert_equal(10, Temperature.new(10.2,'F').as('F',0))
    assert_equal(10.2, Temperature.new(10.2,'F').as('F',2))
  end
  def test_to_s
    assert_equal("283.15K", Temperature.new(10,'C').to_s)
    assert_equal("10.0C", Temperature.new(10,'C').to_s('C'))
  end
end

One last remark: See also the SO-question Units of measurement conversion gem. The ruby-units-gem supports also temperatures.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.