[Tutor] How to refactor a simple, straightforward script into a "proper" program?

[boB Stepp]

Thank you, Joel, for your comments.  I have found them valuable.

On first reading, the bulk of your comments are about code
documentation, and, I believe, expose weaknesses in my understanding
of what is expected and required.  After I complete this initial
reaction to your comments, I am going to hit the 'Net and review
docstrings and code commenting and then return with some code updates
to see if my understanding has improved.  One thing that struck me on
my initial read is that while I was trying to refactor the original
simple script into something more robust, I was still retaining a
"simple script" mentality for my code documentation.  I expect this is
a faulty habit I have fallen into from not having any oversight of my
coding efforts (Other than my questions on this list.).

One question I do wish to ask up front:  _If_ type annotations are
used, does this affect how the code is documented?  My impression is
that with my code as is (no type annotations) I should have added a
section for each function explaining the expected type and function of
each parameter and value returned.  But if I do use type annotations,
that would seem redundant.

On Sat, Jan 4, 2020 at 3:33 PM Joel Goldstick <joel.goldstick at gmail.com> wrote:

> On Sat, Jan 4, 2020 at 2:11 AM boB Stepp <robertvstepp at gmail.com> wrote:

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

My intent was to continue looping, warning the user of his/her error,
until the user enters syntactically valid input, and only then
returning the input for further validation.  Should I have approached
this differently?

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

I did restate the obvious, didn't I?  I know I am supposed to have
some sort of meaningful one-liner to start the docstring before
providing more detailed information below the opening line.  I am
obviously struggling on what to put on this summary line.

> Also, get_input_parameters has no arguments.  and it either returns
> something, or it returns nothing.

Again, I am looping until the user provides logically valid input
before considering the input valid and returning it.  Same question:
Should I have done this differently?

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

Duly noted.

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

I am not sure I am comprehending your point.  Are you saying that the
above function, display_result(pages_per_day), should not have passed
in, pages_per_day, but instead called calc_pages_per_day(), and then
printed that returned result?  If this is correct, what are the
significant differences between the two approaches?  This is what I am
not seeing.

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

That does seem simpler and clearer.  Thanks.

> > if __name__ == "__main__":
> >     main()
> > ============================================================================



-- 
boB