[Tutor] Feedback on coding style

[Alan Gauld]

On 09/05/2022 13:01, Leam Hall wrote:

> import argparse
> from datetime import datetime
> import os.path
> 
> def array_from_file(report_file):
>    data = []
>    with open(report_file, 'r') as file:
>      for line in file:
>        line.strip()
>        datum = line.split()
>        if len(datum) == 4:
>          data.append(datum)
>        else:
>          continue

else: continue is not needed.
The loop will restart without it.

> def report(report_data):
>    highest_systolic  = 0
>    highest_diastolic = 0
>    highest_pulse     = 0
>    latest            = -1.0
>    for datum in report_data:
>      systolic  = int(datum[0])
>      diastolic = int(datum[1])
>      pulse     = int(datum[2])
>      date      = float(datum[3])
>      if systolic > highest_systolic:
>        highest_systolic = systolic
>        highest_systolic_event = datum
>      if diastolic > highest_diastolic:
>        highest_diastolic = diastolic
>        highest_diastolic_event = datum
>      if pulse > highest_pulse:
>        highest_pulse = pulse
>        highest_pulse_event = datum
>      if date > latest:
>        latest_record = datum

OK down to here, but you should return the calculated values as a tuple

> 
>    print("Highest Systolic: {}/{} {} {}".format(*highest_systolic_event))
>    print("Highest Diastolic: {}/{} {} {}".format(*highest_diastolic_event))
>    print("Highest Pulse: {}/{} {} {}".format(*highest_pulse_event))
>    print("Latest Record: {}/{} {} {}".format(*latest_record))

The print calls should be outside the function.
Keep logic and presentation separate in case you
decide to change UI to a GUI, Web page, curses etc.

> def result_string(report_list):
>    return "{} {} {} {}".format(*report_list)

Seems overkill to have this inside a function.

> if args.add:
>    # This format allows sequencing now and parsing later.
>    timestamp   = datetime.now().strftime("%Y%m%d.%H%M")
>    this_report = args.add
>    this_report.append(timestamp)

This is extremely fragile. You really should check the format
and data in add before adding it to your precious daa, it
could end up corrupting the file if badly formed - for example
a float rather than an int value?.

>    with open(report_file, 'a') as file:
>      file.write(result_string(this_report) + "\n")

Is that really much clearer than:

      file.write("{} {} {} {}\n.format(*this_report))

> else:
>    # Default behavior is to report.
>    if os.path.exists(report_file):
>      try:
>        report_data = array_from_file(report_file)
>        report(report_data)
>      except:
>        print("Error processing report data")
>    else:
>      print("Cannot find ", report_file)


-- 
Alan G
Author of the Learn to Program web site
http://www.alan-g.me.uk/
http://www.amazon.com/author/alan_gauld
Follow my photo-blog on Flickr at:
http://www.flickr.com/photos/alangauldphotos