Converting between Fahrenheit and Celsius

I'm learning Ruby and working through some exercises. I completed the following exercise without truly understanding why and how it works. I know that's an embarrassing thing to say but I feel like my subconscious mind wrote the code and conscious me (or maybe unconscious me you could say) is having trouble understanding why and how it works and if it's even written correctly.

I would appreciate if someone can run through this code and give me a breakdown of how it works and if you think it's written well.

My Code

class Temperature

  class << self
    def from_fahrenheit temp
      Temperature.new({f: temp})
    end

    def from_celsius temp
      Temperature.new({c: temp})
    end
  end

  def initialize(options={})
    @f = options[:f]
    @c = options[:c]
  end

  def in_fahrenheit
    return @f if @f
    (@c * (9.0 / 5.0)) + 32
  end

  def in_celsius
    return @c if @c
    (@f - 32) * 5.0 / 9.0
  end
end

class Celsius < Temperature
  def initialize temp
    super(c: temp)
  end
end

class Fahrenheit < Temperature
  def initialize temp
    super(f: temp)
  end
end

Rspec Test

```
describe Temperature do

describe "can be constructed with an options hash" do
describe "in degrees fahrenheit" do
it "at 50 degrees" do
Temperature.new(:f => 50).in_fahrenheit.should == 50
end

describe "and correctly convert to celsius" do
it "at freezing" do
Temperature.new(:f => 32).in_celsius.should == 0
end

it "at boiling" do
Temperature.new(:f => 212).in_celsius.should == 100
end

it "at body temperature" do
Temperature.new(:f => 98.6).in_celsius.should == 37
end

it "at an arbitrary temperature" do
Temperature.new(:f => 68).in_celsius.should == 20
end
end
end

describe "in degrees celsius" do
it "at 50 degrees" do
Temperature.new(:c => 50).in_celsius.should == 50
end

describe "and correct

For someone who wasn't conscious when he wrote this code, it looks quite nice :)

Some observations:

Tests should do only one thing

This is OK in most of your tests, but you got a little 'lazy' at the last two tests - make a different test for each should. You could use RSpec's one-liner syntax, just for the fun of it:

describe "in degrees celsius" do
   subject { Temperature.from_celsius(50) }

   its (:in_celsius) { should == 50 }
   its (:in_fahrenheit) { should == 122 }
 end

Meaningful Names

Again, in most cases, you are doing fine, and there may even be good arguments to say that @f and @c are clear enough in context, but there really is no reason not to call them @fahrenheit and @celsius. temp is also a quirk you should fix...

When things don't change

After you create a Temperature instance, its celsius and fahrenheit values will never change, so there is no reason why you should continue to calculate them - simply store the result in the appropriate member:

def in_fahrenheit
    return @f if @f
    @f = (@c * (9.0 / 5.0)) + 32
  end

You can even make this shorter, using the ||= operator, which will compute and assign the member if it is not assigned:

def in_fahrenheit
    @f ||= (@c * (9.0 / 5.0)) + 32
  end

This is looking more and more like a getter function, and really, since it calls for the state of the instance, you don't need to imply there is ever a calculation, simply call it fahrenheit:

def fahrenheit
  @fahrenheit ||= (@celsius * (9.0 / 5.0)) + 32
end

Have a consistent style

In your code you call methods with hashes with three different styles:

Temperature.new({f: temp})
super(c: temp)
Temperature.new(:c => 50).in_celsius.should == 50

All of these styles work, but it is better to choose one, and stick with it. When using a hash in method, my favorite is the second style (Temperature.new(f: temp))

Use language features

Since Ruby 2.0, named parameters are supported, and you are encouraged to use it. It will give you free validation, as any parameter which is not one of those you declared, an error will be issued:

def initialize(celsius: nil, fahrenheit: nil)
  @fahrenheit = fahrenheit
  @celsius = celsius
end

One last point, regarding your Celsius and Fahrenheit classes, I personally don't see any utility in having them, since they add no logic besides a construction convenience, and may even confuse, since their name implies they support only one scale or the other...

So now, your class will look like this:

class Temperature

  class << self
    def from_fahrenheit fahrenheit
      Temperature.new(fahrenheit: fahrenheit)
    end

    def from_celsius celsius
      Temperature.new(celsius: celsius)
    end
  end

  def initialize(celsius: nil, fahrenheit: nil)
    @fahrenheit = fahrenheit
    @celsius = celsius
  end

  def fahrenheit
    @fahrenheit ||= (@celsius * (9.0 / 5.0)) + 32
  end

  def celsius
    @celsius ||= (@fahrenheit - 32) * 5.0 / 9.0
  end
end

StackExchange Code Review Q#43431, answer score: 6