[Tutor] How to refactor a simple, straightforward script into a "proper" program?
Joel Goldstick
joel.goldstick at gmail.com
Sat Jan 4 16:33:04 EST 2020
I'll chime in. My comments interleaved with your code
On Sat, Jan 4, 2020 at 2:11 AM boB Stepp <robertvstepp at gmail.com> wrote:
>
> I now believe that I have a finalized version2.py that I am satisfied
> with. I have not done the optional type annotations and am in
> progress adding tests. I may have some questions about the resulting
> tests once I complete them. But for my New Year's Python 3 exercise,
> I wish to throw out the below code for code review by anyone who
> wishes to assist boB on his ongoing learning journey. Let me know of
> *anything* that you think I should have done better or more
> professionally or in a more Pythonic style, etc.
>
> Another goal I had hoped to accomplish, but have apparently failed at,
> was to draw in fellow Python learners into the discussion. So many
> times we are asked on this list what can we do to learn how to program
> in Python, and the oft-recommended advice is to come up with a project
> and code it, asking for help as necessary. I thought showing a simple
> program and working on improving it might be helpful for other
> learners. Of course the end result to this point has grown from an
> original 23 LOC to 97 LOC (including blank lines), so have I really
> improved the original product? I await the scores from the judges!
>
> <final version2.py>
> ============================================================================
> #!/usr/bin/env python3
> """Calculate the number of pages per day needed to read a book by a
> given date."""
>
> from datetime import date
>
>
The above looks fine to me. You may want to say what you want the
user to input in order to accomplish this, and what is valid for them
to input.
> def get_input(str_converter, msg):
> """Prompt user with a message and return user input with the
> correct data type."""
> while True:
> try:
> return str_converter(input(msg))
> except ValueError:
> if str_converter == int:
> print("\nPlease enter a whole number!")
> elif str_converter == date.fromisoformat:
> print("\nPlease enter a valid date in the following
> format yyyy-mm-dd!")
>
This one baffles me a little. If str_converter(input(msg)) doesn't
produce a ValueError exception, it returns data, otherwise it prints
something, and returns nothing.
>
> def get_input_params():
> """Collect all needed input parameters."""
> while True:
> str_converters = [int, int, date.fromisoformat]
> input_msgs = [
> "How many pages are there in your book? ",
> "How many pages have you read? ",
> "What is your date to finish the book (yyyy-mm-dd)? ",
> ]
> num_pages_in_book, num_pages_read, goal_date = map(
> get_input, str_converters, input_msgs
> )
> input_params = num_pages_in_book, num_pages_read, goal_date
> if validate_input_params(*input_params):
> return input_params
> print()
>
Your docstring is useless. A docstring should describe what the code
requires coming in, what it returns, and what function it performs.
Its not useful to say something like you have: it does what it needs
to do?
Also, get_input_parameters has no arguments. and it either returns
something, or it returns nothing.
>
> def validate_input_params(num_pages_in_book, num_pages_read, goal_date):
> """Verify input parameters for logical correctness."""
> valid_input = True
> print()
> if num_pages_in_book <= 0:
> print("The number of pages in a book must be a positive integer!")
> valid_input = False
> if num_pages_read < 0:
> print("The number of pages read must be a whole number!")
> valid_input = False
> if num_pages_read == num_pages_in_book and num_pages_in_book > 0:
> print(
> "You have already read the entire book! Why are you
> running this program?"
> )
> if num_pages_read > num_pages_in_book:
> print("You cannot have read more pages than there are in the book!")
> valid_input = False
> if goal_date < date.today():
> print("You cannot set a goal date in the past!")
> valid_input = False
> return valid_input
>
Again, you are too general in your docstring. And you don't spell out
what this thing returns
>
> def calc_pages_per_day(num_pages_in_book, num_pages_read, goal_date):
> """Return number of pages to read each day to attain goal."""
> COUNT_TODAY = 1
> days_to_goal = (goal_date - date.today()).days
> pages_per_day = (num_pages_in_book - num_pages_read) /
> (days_to_goal + COUNT_TODAY)
> return pages_per_day
>
This one is ok i guess
>
> def display_result(pages_per_day):
> """Display how many pages per day the reader must read to finish
> the book."""
> if 0 < pages_per_day <= 1:
> word_to_display = "page"
> else:
> word_to_display = "pages"
> print(
> "You must read {0:.2n} {1} per day to reach your goal.".format(
> pages_per_day, word_to_display
> )
> )
>
I would prefer that I do prints with values returned from functions,
rather than have the function print some value.
>
> def main():
> """Run program and display results."""
> print(
> 'Welcome to "HOW MANY PAGES???"\n'
> "Follow on-screen directions to determine how many pages to
> read each day to "
> "finish your book.\n\n"
> )
> while True:
> input_params = get_input_params()
> display_result(calc_pages_per_day(*input_params))
> run_again = input("Enter 'q' to quit or anything else to continue: ")
> if run_again.lower().startswith("q"):
> break
>
This might be simplified like:
while True:
display_results(calc_pages_per_day(*get_input_params())
if input("bla bla").lower().startswith('q'):
break;
>
> if __name__ == "__main__":
> main()
> ============================================================================
>
> boB
> _______________________________________________
> Tutor maillist - Tutor at python.org
> To unsubscribe or change subscription options:
> https://mail.python.org/mailman/listinfo/tutor
--
Joel Goldstick
http://joelgoldstick.com/blog
http://cc-baseballstats.info/stats/birthdays
More information about the Tutor
mailing list