Python 3 - simple temperature program version 1.3Python 3 - simple temperature programTemperature conversion in CPython Unit Coversion Code Optimization in terms of Space and Time ComplexityGenerating formatted multiplication tables in C++Simple temperature converter in CSimple Temperature Converter 2 in CTemperature converter programTemperature conversion codeXOR cipher C programMedian of a Vector assignmentPython 3 - simple temperature program
How to efficiently lower your karma in fallout 3
Row vectors and column vectors (Mathematica vs Matlab)
Renting a house to a graduate student in my department
How does weapons training transfer to empty hand?
How can I make parentheses stick to formula?
What are these round pads on the bottom of a PCB?
how to find out if there's files in a folder and exit accordingly (in KSH)
Origins of the "array like" strings in BASIC
How is it possible for this circuit to continue functioning correctly?
How long can fsck take on a 30 TB volume?
Is it safe to keep the GPU on 100% utilization for a very long time?
Pre-1993 comic in which Wolverine's claws were turned to rubber?
How is Arya still alive?
Program for finding longest run of zeros from a list of 100 random integers which are either 0 or 1
Is there a need for better software for writers?
Is every story set in the future "science fiction"?
What replaces x86 intrinsics for C when Apple ditches Intel CPUs for their own chips?
Is there any evidence to support the claim that the United States was "suckered into WW1" by Zionists, made by Benjamin Freedman in his 1961 speech
Employee is self-centered and affects the team negatively
What's an appropriate age to involve kids in life changing decisions?
Can the president of the United States be guilty of insider trading?
Why do the Avengers care about returning these items in Endgame?
Why Faces eat each other?
Lorentz invariance of Maxwell's equations in matter
Python 3 - simple temperature program version 1.3
Python 3 - simple temperature programTemperature conversion in CPython Unit Coversion Code Optimization in terms of Space and Time ComplexityGenerating formatted multiplication tables in C++Simple temperature converter in CSimple Temperature Converter 2 in CTemperature converter programTemperature conversion codeXOR cipher C programMedian of a Vector assignmentPython 3 - simple temperature program
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;
$begingroup$
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()
python beginner python-3.x unit-conversion
New contributor
J Karle is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
$endgroup$
add a comment |
$begingroup$
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()
python beginner python-3.x unit-conversion
New contributor
J Karle is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
$endgroup$
add a comment |
$begingroup$
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()
python beginner python-3.x unit-conversion
New contributor
J Karle is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
$endgroup$
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()
python beginner python-3.x unit-conversion
python beginner python-3.x unit-conversion
New contributor
J Karle is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
New contributor
J Karle is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
edited May 5 at 19:46
200_success
132k20159425
132k20159425
New contributor
J Karle is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
asked May 5 at 19:11
J KarleJ Karle
9610
9610
New contributor
J Karle is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
New contributor
J Karle is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
add a comment |
add a comment |
3 Answers
3
active
oldest
votes
$begingroup$
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:
- Asking and validating user input.
- Handling how to convert the input.
- 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')
$endgroup$
$begingroup$
@Peilonrazy Thank you for the feedback; excellent as always. I am clearly not fully understanding what recursion and iteration is. However, I am researching that now, and once I have done so I will come back with a few questions, or a new version of the program. One question now however - WRT converting the input to a float in the input definition, on top of asking for the input, why is this "bad?" It has to be converted somewhere, so whether is done in that def when it is entered, or back in the main() def, what are the advantages or disadvantages?
$endgroup$
– J Karle
2 days ago
1
$begingroup$
@JKarle 1. If you are only allowed to look at themain, it's hard to tell what the code is doing in your method. However when it's written the way I suggested it's immediately apparent what it does. And sinceinput_floathas a good name you can reasonably guess what the code is doing just from themainmethod. 2. If you need to later get a floating point number from the user, but you don't want either of the convert functions to run you either; a. have to make a new function b. hack at the existing function. And so it's better to keep SRP.
$endgroup$
– Peilonrayz
2 days ago
add a comment |
$begingroup$
@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.
$endgroup$
$begingroup$
Thank you, I was thinking about that, but forgot to follow-up and look it up. I will take that for action!
$endgroup$
– J Karle
2 days ago
add a comment |
$begingroup$
Your conversion functions have two notable things:
You're printing the result directly. Don't do this. For toy programs like this doing so doesn't create problems. 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. What if you wanted to send the raw data over the internet or save it to a file?
You're putting an empty
returnat the end. This is redundant though and unnecessary.Noneis automatically returned at the end anyways if noreturnis met, which is equivalent to what you're doing.
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:
# Save if we're converting to Celsius or not
is_celsius = selection == 'c'
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.
$endgroup$
$begingroup$
Thank you for the feedback. Copy on the empty return (that makes sense), and that I should not print the result directly (that also makes sense after thinking on it for the night.) Rather, and fixing both issues raised, I should calculate the value and return that from the def, to then be display as required.
$endgroup$
– J Karle
2 days ago
1
$begingroup$
@JKarle Yes, let the caller decide how they want to use the data. If they want to print it out, they can do that themselves. Don't force data to be used in a certain way we possible unless you have good reason. You'll limit how easily that function can be used by other code in the future.
$endgroup$
– Carcigenicate
2 days ago
add a comment |
Your Answer
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
J Karle is a new contributor. Be nice, and check out our Code of Conduct.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f219767%2fpython-3-simple-temperature-program-version-1-3%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
3 Answers
3
active
oldest
votes
3 Answers
3
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
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:
- Asking and validating user input.
- Handling how to convert the input.
- 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')
$endgroup$
$begingroup$
@Peilonrazy Thank you for the feedback; excellent as always. I am clearly not fully understanding what recursion and iteration is. However, I am researching that now, and once I have done so I will come back with a few questions, or a new version of the program. One question now however - WRT converting the input to a float in the input definition, on top of asking for the input, why is this "bad?" It has to be converted somewhere, so whether is done in that def when it is entered, or back in the main() def, what are the advantages or disadvantages?
$endgroup$
– J Karle
2 days ago
1
$begingroup$
@JKarle 1. If you are only allowed to look at themain, it's hard to tell what the code is doing in your method. However when it's written the way I suggested it's immediately apparent what it does. And sinceinput_floathas a good name you can reasonably guess what the code is doing just from themainmethod. 2. If you need to later get a floating point number from the user, but you don't want either of the convert functions to run you either; a. have to make a new function b. hack at the existing function. And so it's better to keep SRP.
$endgroup$
– Peilonrayz
2 days ago
add a comment |
$begingroup$
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:
- Asking and validating user input.
- Handling how to convert the input.
- 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')
$endgroup$
$begingroup$
@Peilonrazy Thank you for the feedback; excellent as always. I am clearly not fully understanding what recursion and iteration is. However, I am researching that now, and once I have done so I will come back with a few questions, or a new version of the program. One question now however - WRT converting the input to a float in the input definition, on top of asking for the input, why is this "bad?" It has to be converted somewhere, so whether is done in that def when it is entered, or back in the main() def, what are the advantages or disadvantages?
$endgroup$
– J Karle
2 days ago
1
$begingroup$
@JKarle 1. If you are only allowed to look at themain, it's hard to tell what the code is doing in your method. However when it's written the way I suggested it's immediately apparent what it does. And sinceinput_floathas a good name you can reasonably guess what the code is doing just from themainmethod. 2. If you need to later get a floating point number from the user, but you don't want either of the convert functions to run you either; a. have to make a new function b. hack at the existing function. And so it's better to keep SRP.
$endgroup$
– Peilonrayz
2 days ago
add a comment |
$begingroup$
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:
- Asking and validating user input.
- Handling how to convert the input.
- 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')
$endgroup$
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:
- Asking and validating user input.
- Handling how to convert the input.
- 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')
answered May 5 at 19:34
PeilonrayzPeilonrayz
27.4k342114
27.4k342114
$begingroup$
@Peilonrazy Thank you for the feedback; excellent as always. I am clearly not fully understanding what recursion and iteration is. However, I am researching that now, and once I have done so I will come back with a few questions, or a new version of the program. One question now however - WRT converting the input to a float in the input definition, on top of asking for the input, why is this "bad?" It has to be converted somewhere, so whether is done in that def when it is entered, or back in the main() def, what are the advantages or disadvantages?
$endgroup$
– J Karle
2 days ago
1
$begingroup$
@JKarle 1. If you are only allowed to look at themain, it's hard to tell what the code is doing in your method. However when it's written the way I suggested it's immediately apparent what it does. And sinceinput_floathas a good name you can reasonably guess what the code is doing just from themainmethod. 2. If you need to later get a floating point number from the user, but you don't want either of the convert functions to run you either; a. have to make a new function b. hack at the existing function. And so it's better to keep SRP.
$endgroup$
– Peilonrayz
2 days ago
add a comment |
$begingroup$
@Peilonrazy Thank you for the feedback; excellent as always. I am clearly not fully understanding what recursion and iteration is. However, I am researching that now, and once I have done so I will come back with a few questions, or a new version of the program. One question now however - WRT converting the input to a float in the input definition, on top of asking for the input, why is this "bad?" It has to be converted somewhere, so whether is done in that def when it is entered, or back in the main() def, what are the advantages or disadvantages?
$endgroup$
– J Karle
2 days ago
1
$begingroup$
@JKarle 1. If you are only allowed to look at themain, it's hard to tell what the code is doing in your method. However when it's written the way I suggested it's immediately apparent what it does. And sinceinput_floathas a good name you can reasonably guess what the code is doing just from themainmethod. 2. If you need to later get a floating point number from the user, but you don't want either of the convert functions to run you either; a. have to make a new function b. hack at the existing function. And so it's better to keep SRP.
$endgroup$
– Peilonrayz
2 days ago
$begingroup$
@Peilonrazy Thank you for the feedback; excellent as always. I am clearly not fully understanding what recursion and iteration is. However, I am researching that now, and once I have done so I will come back with a few questions, or a new version of the program. One question now however - WRT converting the input to a float in the input definition, on top of asking for the input, why is this "bad?" It has to be converted somewhere, so whether is done in that def when it is entered, or back in the main() def, what are the advantages or disadvantages?
$endgroup$
– J Karle
2 days ago
$begingroup$
@Peilonrazy Thank you for the feedback; excellent as always. I am clearly not fully understanding what recursion and iteration is. However, I am researching that now, and once I have done so I will come back with a few questions, or a new version of the program. One question now however - WRT converting the input to a float in the input definition, on top of asking for the input, why is this "bad?" It has to be converted somewhere, so whether is done in that def when it is entered, or back in the main() def, what are the advantages or disadvantages?
$endgroup$
– J Karle
2 days ago
1
1
$begingroup$
@JKarle 1. If you are only allowed to look at the
main, it's hard to tell what the code is doing in your method. However when it's written the way I suggested it's immediately apparent what it does. And since input_float has a good name you can reasonably guess what the code is doing just from the main method. 2. If you need to later get a floating point number from the user, but you don't want either of the convert functions to run you either; a. have to make a new function b. hack at the existing function. And so it's better to keep SRP.$endgroup$
– Peilonrayz
2 days ago
$begingroup$
@JKarle 1. If you are only allowed to look at the
main, it's hard to tell what the code is doing in your method. However when it's written the way I suggested it's immediately apparent what it does. And since input_float has a good name you can reasonably guess what the code is doing just from the main method. 2. If you need to later get a floating point number from the user, but you don't want either of the convert functions to run you either; a. have to make a new function b. hack at the existing function. And so it's better to keep SRP.$endgroup$
– Peilonrayz
2 days ago
add a comment |
$begingroup$
@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.
$endgroup$
$begingroup$
Thank you, I was thinking about that, but forgot to follow-up and look it up. I will take that for action!
$endgroup$
– J Karle
2 days ago
add a comment |
$begingroup$
@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.
$endgroup$
$begingroup$
Thank you, I was thinking about that, but forgot to follow-up and look it up. I will take that for action!
$endgroup$
– J Karle
2 days ago
add a comment |
$begingroup$
@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.
$endgroup$
@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.
answered May 5 at 19:50
AlexVAlexV
1,915720
1,915720
$begingroup$
Thank you, I was thinking about that, but forgot to follow-up and look it up. I will take that for action!
$endgroup$
– J Karle
2 days ago
add a comment |
$begingroup$
Thank you, I was thinking about that, but forgot to follow-up and look it up. I will take that for action!
$endgroup$
– J Karle
2 days ago
$begingroup$
Thank you, I was thinking about that, but forgot to follow-up and look it up. I will take that for action!
$endgroup$
– J Karle
2 days ago
$begingroup$
Thank you, I was thinking about that, but forgot to follow-up and look it up. I will take that for action!
$endgroup$
– J Karle
2 days ago
add a comment |
$begingroup$
Your conversion functions have two notable things:
You're printing the result directly. Don't do this. For toy programs like this doing so doesn't create problems. 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. What if you wanted to send the raw data over the internet or save it to a file?
You're putting an empty
returnat the end. This is redundant though and unnecessary.Noneis automatically returned at the end anyways if noreturnis met, which is equivalent to what you're doing.
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:
# Save if we're converting to Celsius or not
is_celsius = selection == 'c'
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.
$endgroup$
$begingroup$
Thank you for the feedback. Copy on the empty return (that makes sense), and that I should not print the result directly (that also makes sense after thinking on it for the night.) Rather, and fixing both issues raised, I should calculate the value and return that from the def, to then be display as required.
$endgroup$
– J Karle
2 days ago
1
$begingroup$
@JKarle Yes, let the caller decide how they want to use the data. If they want to print it out, they can do that themselves. Don't force data to be used in a certain way we possible unless you have good reason. You'll limit how easily that function can be used by other code in the future.
$endgroup$
– Carcigenicate
2 days ago
add a comment |
$begingroup$
Your conversion functions have two notable things:
You're printing the result directly. Don't do this. For toy programs like this doing so doesn't create problems. 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. What if you wanted to send the raw data over the internet or save it to a file?
You're putting an empty
returnat the end. This is redundant though and unnecessary.Noneis automatically returned at the end anyways if noreturnis met, which is equivalent to what you're doing.
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:
# Save if we're converting to Celsius or not
is_celsius = selection == 'c'
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.
$endgroup$
$begingroup$
Thank you for the feedback. Copy on the empty return (that makes sense), and that I should not print the result directly (that also makes sense after thinking on it for the night.) Rather, and fixing both issues raised, I should calculate the value and return that from the def, to then be display as required.
$endgroup$
– J Karle
2 days ago
1
$begingroup$
@JKarle Yes, let the caller decide how they want to use the data. If they want to print it out, they can do that themselves. Don't force data to be used in a certain way we possible unless you have good reason. You'll limit how easily that function can be used by other code in the future.
$endgroup$
– Carcigenicate
2 days ago
add a comment |
$begingroup$
Your conversion functions have two notable things:
You're printing the result directly. Don't do this. For toy programs like this doing so doesn't create problems. 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. What if you wanted to send the raw data over the internet or save it to a file?
You're putting an empty
returnat the end. This is redundant though and unnecessary.Noneis automatically returned at the end anyways if noreturnis met, which is equivalent to what you're doing.
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:
# Save if we're converting to Celsius or not
is_celsius = selection == 'c'
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.
$endgroup$
Your conversion functions have two notable things:
You're printing the result directly. Don't do this. For toy programs like this doing so doesn't create problems. 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. What if you wanted to send the raw data over the internet or save it to a file?
You're putting an empty
returnat the end. This is redundant though and unnecessary.Noneis automatically returned at the end anyways if noreturnis met, which is equivalent to what you're doing.
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:
# Save if we're converting to Celsius or not
is_celsius = selection == 'c'
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.
edited May 5 at 23:25
answered May 5 at 19:44
CarcigenicateCarcigenicate
4,57911735
4,57911735
$begingroup$
Thank you for the feedback. Copy on the empty return (that makes sense), and that I should not print the result directly (that also makes sense after thinking on it for the night.) Rather, and fixing both issues raised, I should calculate the value and return that from the def, to then be display as required.
$endgroup$
– J Karle
2 days ago
1
$begingroup$
@JKarle Yes, let the caller decide how they want to use the data. If they want to print it out, they can do that themselves. Don't force data to be used in a certain way we possible unless you have good reason. You'll limit how easily that function can be used by other code in the future.
$endgroup$
– Carcigenicate
2 days ago
add a comment |
$begingroup$
Thank you for the feedback. Copy on the empty return (that makes sense), and that I should not print the result directly (that also makes sense after thinking on it for the night.) Rather, and fixing both issues raised, I should calculate the value and return that from the def, to then be display as required.
$endgroup$
– J Karle
2 days ago
1
$begingroup$
@JKarle Yes, let the caller decide how they want to use the data. If they want to print it out, they can do that themselves. Don't force data to be used in a certain way we possible unless you have good reason. You'll limit how easily that function can be used by other code in the future.
$endgroup$
– Carcigenicate
2 days ago
$begingroup$
Thank you for the feedback. Copy on the empty return (that makes sense), and that I should not print the result directly (that also makes sense after thinking on it for the night.) Rather, and fixing both issues raised, I should calculate the value and return that from the def, to then be display as required.
$endgroup$
– J Karle
2 days ago
$begingroup$
Thank you for the feedback. Copy on the empty return (that makes sense), and that I should not print the result directly (that also makes sense after thinking on it for the night.) Rather, and fixing both issues raised, I should calculate the value and return that from the def, to then be display as required.
$endgroup$
– J Karle
2 days ago
1
1
$begingroup$
@JKarle Yes, let the caller decide how they want to use the data. If they want to print it out, they can do that themselves. Don't force data to be used in a certain way we possible unless you have good reason. You'll limit how easily that function can be used by other code in the future.
$endgroup$
– Carcigenicate
2 days ago
$begingroup$
@JKarle Yes, let the caller decide how they want to use the data. If they want to print it out, they can do that themselves. Don't force data to be used in a certain way we possible unless you have good reason. You'll limit how easily that function can be used by other code in the future.
$endgroup$
– Carcigenicate
2 days ago
add a comment |
J Karle is a new contributor. Be nice, and check out our Code of Conduct.
J Karle is a new contributor. Be nice, and check out our Code of Conduct.
J Karle is a new contributor. Be nice, and check out our Code of Conduct.
J Karle is a new contributor. Be nice, and check out our Code of Conduct.
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f219767%2fpython-3-simple-temperature-program-version-1-3%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown