5
\$\begingroup\$

I'm learning Ruby and as an exercise I decided to build this little temperature converter app (C to F, F to C, F to K, etc). Looking back at it, it's pretty much just a bunch of boilerplate code. Any ideas on how I could shorten this up and make it a little more dynamic would really be appreciated!

#!/usr/bin/ruby
# My project: temperature converter (temp_conv.rb)
# start date: 10-26-2016
# finish date:
# A simple program to convert either from C to F, or F to C depending on the user's choice.

def c_to_f(c)
    ans = c*1.8+32
    puts "#{c}C = #{ans}F"
end

def f_to_c(f)
    ans = (f-32)/1.8
    puts "#{f}F = #{ans}C"
end

def f_to_k(f)
    ans = (f+459.65)*5/9
    puts "#{f}F = #{ans}K"
end

def k_to_f(k)
    ans = k * 9/5 - 459.67
    puts "#{k}K = #{ans}F"
end

def c_to_k(c)
    ans = c + 273.15
    puts "#{c}C = #{ans}K"
end

def k_to_c(k)
    ans = k - 273.15
    puts "#{k}K = #{ans}C"
end

again = nil
while again != "n" 
    puts "welcome to Temp Converter 1.0!
          Please select an option from the following menu:
    1) Convert Celsius to Fahrenheit
    2) Convert Fahrenheit to Celsius
    3) Convert Fahrenheit to Kelvin
    4) Convert Kelvin to Fahrenheit
    5) Convert Celsius to Kelvin
    6) Convert Kelvin to Celsius
    0) Quit
    "
    m_choice = nil
    loop do
        print ":> "
        m_choice = gets.chomp.to_i
        break if m_choice >= 0 && m_choice <= 6
        puts "Invalid input. Please enter a choice between 0 and 6."
    end
    case m_choice
    when 1
        puts "Celsius to Fahrenheit"
        print "enter Temperature:> "
        cel = gets.chomp.to_i
        c_to_f(cel)
    when 2
        puts "Fahrenheit to Celsius"
        print "Enter Temperature:> "
        fah = gets.chomp.to_i
        f_to_c(fah)
    when 3
        puts "Fahrenheit to Kelvin"
        print "Enter Temperature:> "
        fah = gets.chomp.to_i
        f_to_k(fah)
    when 4
        puts "Kelvin to Fahrenheit"
        print "Enter Temperature:> "
        kel = gets.chomp.to_i
        k_to_f(kel)
    when 5
        puts "Celsius to Kelvin"
        print "Enter Temperature:> "
        cel = gets.chomp.to_i
        c_to_k(cel)
    when 6
        puts "Kelvin to Celsius"
        print "Enter Temperature:> "
        kel = gets.chomp.to_i
        k_to_c(kel)
    else 
        puts "Thank you for using, Temperature Converter!"
        sleep(1)
        exit(0)
    end

    print "would you like to run again? (Y/N)"
    loop do
        print ":> "
        again = gets.chomp.downcase
        break if again == 'y' || again == 'n'
        puts "Invalid Input. Please choose either 'y' or 'n'"
    end
end
\$\endgroup\$
1
\$\begingroup\$

The good

  • Short, focused methods. Methods should be short and have a single responsibility. With your conversion related methods, you've achieved this - nice job. Ruby especially encourages small methods; it's common to see them be as short as a single line.
  • It works. Your program appears to correctly convert between temperatures.
  • It's readable. You followed reasonably good formatting and style guidelines. Note that conventional Ruby style uses 2 spaces rather then tabs for indentation - I recommend adopting this.

Suggestions

One way you might shorten things up and dynamicize is as follows: rather than having 6 methods handling each conversion, you could cut it in half by writing functions such as this:

def convert_to_fahrenheit(start_unit, temp)
  if start_unit == 'C'
    return temp * 1.8 + 32
  elsif start_unit == 'K'
    return temp * 9 / 5 - 459.67
  else
    raise ArgumentError, "Unit: #{start_unit} is invalid. Unit should only be K or C"
  end
end

With this solution, the tradeoff is that our methods get bigger but they're still reasonably short and each still has a single responsibility.

Of course with this solution you have to change the existing 1-6 numbery way of gathering input. Instead you might have methods like:

def print_starting_unit_request
  puts "Enter the temperature unit you have (C, F, K):"
end

def print_resulting_unit_request
  puts "Enter the temperature unit you want (C, F, K):"
end

def get_temperature_unit_input
  gets.chomp.upcase
end

def valid_input_temperature?(input)
  ['K', 'C', 'F'].include? input
end

def units_match?(starting_unit, resulting_unit)
  starting_unit == resulting_unit
end

This would eliminate the long series of repetitive case statements. You might have something like:

def convert_temperature(start_unit, end_unit, temp)
  if end_unit == 'C'
    convert_to_celcius(start_unit, temp)
  elsif end_unit == 'K'
    convert_to_kelvin(start_unit, temp)
  elsif end_unit == 'F'
    convert_to_fahrenheit(start_unit, temp)
  else
    raise ArgumentError, "Unit: #{end_unit} is invalid. Valid (K, C, F)."
  end
end

Do note that this is not an ideal solution. It is preferable to use an object-oriented class based solution rather than conditionals; I get the sense that you might not have learned OOP yet. This would be a good exercise in OOP by refactoring once you get more comfortable.

These are rough ideas to get you going and of course you'd have to set up the logic appropriately (looping, etc). A key idea here is that you want to be using methods often. Methods are self documenting and make code more readable.


One thing I noticed when looking at:

**1) Convert Celsius to Fahrenheit**
2) Convert Fahrenheit to Celsius
3) Convert Fahrenheit to Kelvin
4) Convert Kelvin to Fahrenheit
**5) Convert Celsius to Kelvin**
6) Convert Kelvin to Celsius
0) Quit

Every other combination of starting temperature seems sensibly grouped together but 1 & 5 unexpectedly are split up. I would have expected 5 to come right after 1. This might not seem like a big deal and it really isn't here but I think it illustrates a couple of important ideas of writing clean code to be mindful of: consistency and don't be arbitrary.

Consistency is often mentioned related to various aspects of writing good code; it's especially important when many people are touching the same software. The essential idea is that when things are consistent, they are more easily read and understood; when they're not, things tend to stick out, distract us, and interrupt the flow of whatever we're trying to do.

Don't be arbitrary is an idea I recently read from Clean Code, a popular book about writing... clean code:

Have a reason for the way you structure your code, and make sure that reason is communi- cated by the structure of the code. If a structure appears arbitrary, others will feel empowered to change it. (Martin, 303)

This may seem overkill for a post like this, but hey, I just finished reading the book and want to help reinforce the concepts. But I think it's a good thing to be aware of no matter what level the programmer. In this situation, we structure our input capturing into intuitive grouping that the user might expect - therefore, it's not arbitrary.


Style note: Prefer spaces between operations (and be consistent). You have both:

ans = c*1.8+32

ans = k * 9/5 - 459.67

Prefer the later.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ An if/elsif chain where the condition applies to the same value may be convert into a case block. \$\endgroup\$ – tokland Oct 28 '16 at 10:10
  • \$\begingroup\$ @tokland Sure. Is case preferred by most Rubyists? I'm not a deeply experienced Ruby dev. \$\endgroup\$ – jsuth Oct 28 '16 at 10:15
  • 1
    \$\begingroup\$ In this case, most devs would write a case, yes. Note that in Ruby a case uses implicitly the special case equality operator, ===, which make it's pretty versatile. More info: skorks.com/2009/08/… \$\endgroup\$ – tokland Oct 28 '16 at 10:39

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.