Mfcttrf

Thank you to everyone for the feedback provided to the initial version of this program posted here.

Please find below the newest version, revised based on the comments provided, for review and comment on how I can further improve my coding.

I have removed the portion of the program that checked if the provided value was a legitimate temperature, as this was useless code only showing that I had recently learned what the opposite of absolute zero was....who cares, and who cares what value the user wants to convert!

I look forward to your feedback.

#!/usr/bin/python
"""

Program: Temperature Conversion (C to F, or F to C)
Date: 05 May 2019
Version: 1.2
Author: Jason P. Karle

Remark: This program was inspired by a Python exercise that
asks you to create a program that will convert one Celsius value to Fahrenheit;
so a program that can be executed with three lines of code.

However, I wanted to make something that would allow the user to
convert to and from either C of F, and do so multiple times, until the user
decides to end the program. This was also an exercise for me to
advance not only my code skills, but how I structure a program.

version history:
 1.0 Initial draft
 1.1 Correction of runtime; posted to StackExchange for feedback
 1.2 Re-coded based on feedback; trying to improve flow control
"""


def read_selection():
 selection = input('''Welcome to the temperature conversion program!

Please make a selection:

 c to convert from Celcius to Fahrenheit;
 f to convert from Fahrenheit to Celsius; or
 q to quit the program.

Enter your selection: ''').lower()
 return selection


def value_input(selection):
 value = input('''nPlease enter the temperature you
want to convert: ''')
 try:
 value = float(value)
 except ValueError:
 print('That is not a number!n')
 else:
 if selection == 'c':
 convert_c2f(value)
 else:
 convert_f2c(value)
 # return value


def convert_c2f(value):
 print(f'The answer is: (value * (9/5)) + 32°Fn')
 return


def convert_f2c(value):
 print(f'The answer is: (value-32) * (5/9)°Cn')
 return


def main():
 while True:
 selection = read_selection()
 if selection == 'q':
 return
 elif selection == 'c' or selection == 'f':
 value_input(selection)
 '''convert_c2f()
 elif selection == 'f':
 convert_f2C()'''
 else:
 print('Invalid selction. Try again.n')


if __name__ == '__main__':
 main()

You're not following PEP8 and you still have a spaghetti mind-set. Each function should have a single responsibility.

value_input however is in charge of:

1. Asking and validating user input.


2. Handling how to convert the input.


3. Convert and display the input.

This should instead only perform the first task I've said above. After this you should have the calling code perform 2 and 3.

convert_c2f also is responsible for two things converting and displaying the input.

It can be seen in main that you were originally doing something better than you have now, so it's unclear why you changed this.

def read_selection():
 return input('''Welcome to the temperature conversion program!

Please make a selection:

 c to convert from Celcius to Fahrenheit;
 f to convert from Fahrenheit to Celsius; or
 q to quit the program.

Enter your selection: ''')


def float_input():
 value = input('''nPlease enter the temperature you
want to convert: ''')
 try:
 return float(value)
 except ValueError:
 print('That is not a number!n')


def convert_c2f(value):
 return (value * (9 / 5)) + 32


def convert_f2c(value):
 return (value - 32) * (5 / 9)


def main():
 while True:
 selection = read_selection().lower()
 if selection == 'q':
 return
 elif selection == 'c' or selection == 'f':
 value = float_input(selection)
 if selection == 'c':
 converted = convert_c2f(value)
 print(f'The answer is: converted°Fn')
 else:
 converted = convert_f2c(value)
 print(f'The answer is: converted°Cn')
 else:
 print('Invalid selction. Try again.n')

Your conversion functions have two notable things:

• You're printing the result directly. Don't do this. For toy programs like this, this is ok. In the real world though, you don't just print data to the screen, you use data. With how you have it now, the caller can't actually use the converted value for anything.




• You're putting an empty return at the end. This is redundant though and unnecessary.

I would have the functions return the converted value, and print it at the call site:

def value_input(selection):
 value = input('''nPlease enter the temperature you
want to convert: ''')
 try:
 value = float(value)

 except ValueError:
 print('That is not a number!n')

 else:
 is_celsius = selection == 'c' # Save if we're converting to Celsius or not

 new_value = convert_c2f(value) if is_celsius else convert_f2c(value)
 unit_str = "C" if is_celsius else "F"

 print(f'The answer is: new_value°unit_str')


def convert_c2f(value):
 return (value * (9 / 5)) + 32

def convert_f2c(value):
 return (value - 32) * (5 / 9)

I decided to reduce the printing down to a single call to print and just decide what data will be printed ahead of time. This is personal style though. I get an odd thrill out of reducing duplication. Do whatever you feel is more readable.

@Peilonrayz and @Carcigenicate have great feedback you should definitely follow.

Since you are using Python 3 features like f-string, #!/usr/bin/env python3 should be used as shebang line.