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;








6












$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()









share|improve this question









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$


















    6












    $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()









    share|improve this question









    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$














      6












      6








      6


      3



      $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()









      share|improve this question









      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






      share|improve this question









      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.










      share|improve this question









      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.








      share|improve this question




      share|improve this question








      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.






















          3 Answers
          3






          active

          oldest

          votes


















          7












          $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:



          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')





          share|improve this answer









          $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 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



















          6












          $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.






          share|improve this answer









          $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



















          4












          $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 return at the end. This is redundant though and unnecessary. None is automatically returned at the end anyways if no return is 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.






          share|improve this answer











          $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











          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.









          draft saved

          draft discarded


















          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









          7












          $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:



          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')





          share|improve this answer









          $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 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
















          7












          $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:



          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')





          share|improve this answer









          $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 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














          7












          7








          7





          $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:



          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')





          share|improve this answer









          $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:



          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')






          share|improve this answer












          share|improve this answer



          share|improve this answer










          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 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$
            @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 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$
          @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














          6












          $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.






          share|improve this answer









          $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
















          6












          $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.






          share|improve this answer









          $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














          6












          6








          6





          $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.






          share|improve this answer









          $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.







          share|improve this answer












          share|improve this answer



          share|improve this answer










          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

















          • $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












          4












          $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 return at the end. This is redundant though and unnecessary. None is automatically returned at the end anyways if no return is 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.






          share|improve this answer











          $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















          4












          $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 return at the end. This is redundant though and unnecessary. None is automatically returned at the end anyways if no return is 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.






          share|improve this answer











          $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













          4












          4








          4





          $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 return at the end. This is redundant though and unnecessary. None is automatically returned at the end anyways if no return is 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.






          share|improve this answer











          $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 return at the end. This is redundant though and unnecessary. None is automatically returned at the end anyways if no return is 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.







          share|improve this answer














          share|improve this answer



          share|improve this answer








          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
















          • $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










          J Karle is a new contributor. Be nice, and check out our Code of Conduct.









          draft saved

          draft discarded


















          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.




          draft saved


          draft discarded














          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





















































          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







          Popular posts from this blog

          Grendel Contents Story Scholarship Depictions Notes References Navigation menu10.1093/notesj/gjn112Berserkeree

          Area configuration aggregation error after install Porto themeMagento 2.1 CE Installed but front/backend not loading/workingCSS not loading on page within Magento 2 pageCannot install module in Magento 2no commands defined in the “setup” namespace. in Magento2Magento 2: Static files are present but shows 404Why do i have to always run the commands to clean cache in Magento 2.1.8?Failure reason: 'Unable to unserialize value.'Error 500 after magento migrationIn production mode the site does not loadMagento 2 : Error 500 after installing

          Middle Expansion Olielle Resaix Definition: Uttering songs of triumph shouting with joy triumphant exulting Sejunction Journal 붙다 달 고급 품목 외출 The stretch trades the screeching tin. Definition: The act of speaking with a drawl a drawl Cough Sand Definition: An uproar a quarrel a noisy outbreak Shake Iron Publicize Horse House Baby 사과 Resaix Flaggy Jelly Temporary Unequaled Puppet A drop in the bucket Shrew 성격 회원 성질 미팅 The burn frames the tacky quality. Materialistic The smoke reduces the way. Yammoe Nondescript Cheek 얼굴 배 약하다 날리다 타다 The illegal country shows the iron. Help Rule Drearien Smoke Teaching Meaty Wasp Abraham Lincoln Jaws 진심 수리하다 Size Cork Idea Convert Think Lark John Lennon 거울 청소 군 추천하다 아이스크림