3
\$\begingroup\$

I wanted to share my code for others to critique. This program is something I started while struggling with homework problems in my Statistics course, and after getting the ball rolling, I decided to keep adding and perfecting it. Right now it is in a usable state, but I want to hear the opinion of other more experienced coders. Coding is something I'm passionate about and I need to know where my knowledge and methodology stands.

import statistics
import sys

samples = []


def menu():
    print("Welcome to the Statistics Assistant 3000")
    print("Select from the following options:")
    print("    1. Find the mean")
    print("    2. Find the median")
    print("    3. Find the mode")
    print("    4. Find the range")
    print("    5. Find the standard deviation")
    print("    6. Find the z-score")
    print("    0. Quit program")

    choice = input("Pick one: ")

    if choice == '1':
        find_mean()
    elif choice == '2':
        find_median()
    elif choice == '3':
        find_mode()
    elif choice == '4':
        find_range()
    elif choice == '5':
        standard_deviation()
    elif choice == '6':
        z_score()
    elif choice == '0':
        f = open('sultry.txt')

        for line in f:
            print(line)
        sys.exit()
    else:
        print("That isn't one of the options")
        print("Try again")
        menu()


def load_samples():
    global samples

    print("Would you like to load samples from a text file")
    print("or enter each sample manually?")
    print("    1. File")
    print("    2. Manually")
    print("    3. Return")

    user_response = input("> ")

    if "1" in user_response:

        # force_open = os.startfile('sample_list.txt')
        # input("Hit enter when done saving data...")

        f = open("sample_list.txt")

        samples = f.readlines()
        samples = [float(x) for x in samples]

        # samples = [int(x) for x in samples]

        return samples

    elif "2" in user_response:

        while True:

            print("Enter a value. Enter 'done' when finished")
            value = input('>')

            try:
                if 'done' in value.lower():
                    print("You entered done")
                    print("Done? [Y/N]")
                    user_response = input(">")

                    if 'Y' or 'y' in user:
                        return samples
                        menu()

                else:
                    value = int(value)
                    samples.append(value)

                    print(samples)

            except:
                print("You need to enter an integer")


    elif "3" in user_response:
        menu()

    else:
        print("I don't recognize that option")


def quit():
    print("Than you for using this application")
    sys.exit()


def find_mean():
    global samples

    load_samples()

    mean = round(statistics.mean(samples), 1)
    print("The mean = " + str(mean))

    print("Return to menu?")
    print("[Y/N]")

    user_input = input(">")
    user_input = user_input.lower()

    if 'y' in user_input:
        menu()
    else:
        quit()


def find_median():
    global samples

    load_samples()

    result = statistics.median(samples)
    print("The median is %s" % (result))

    print("Return to menu?")
    print("[Y/N]")

    user_input = input(">")
    user_input = user_input.lower()

    if 'y' in user_input:
        menu()
    else:
        quit()


def find_mode():
    global samples

    load_samples()

    result = statistics.mode(samples)
    print("The mode is %s" % (result))

    print("Return to menu?")
    print("[Y/N]")

    user_input = input(">")
    user_input = user_input.lower()

    if 'y' in user_input:
        menu()
    else:
        quit()


def find_range():
    global samples

    load_samples()

    x = min(samples)
    y = max(samples)

    result = (y - x)

    print("The range of %s and %s is %s" % (x, y, result))

    print("Return to menu?")
    print("[Y/N]")

    user_input = input(">")
    user_input = user_input.lower()

    if 'y' in user_input:
        menu()
    else:
        quit()


def standard_deviation():
    global samples
    load_samples()

    stan_dev = statistics.stdev(samples)
    print('{:.03}'.format(stan_dev))

    print("Return to menu?")
    print("[Y/N]")

    user_input = input(">")
    user_input = user_input.lower()

    if 'y' in user_input:
        menu()
    else:
        quit()


def z_score():
    global samples
    load_samples()

    stan_dev = statistics.stdev(samples)
    mean = statistics.mean(samples)
    middle = statistics.median(samples)

    z = round(((middle - mean) / stan_dev), 2)

    print("The Z-Score is " + str(z))

    print("Return to menu?")
    print("[Y/N]")
    user_input = input(">")
    user_input = user_input.lower()

    if 'y' in user_input:
        menu()
    else:
        quit()


if __name__ == '__main__':
    try:
        menu()
    except:
        print("Press ENTER to exit...")
        input()
\$\endgroup\$
7
\$\begingroup\$

You can use dictionary for making menu

def menu():
    prompt = '''Welcome to the Statistics Assistant 3000
    Select from the following options:
    1. Find the mean
    2. Find the median
    3. Find the mode
    4. Find the range
    5. Find the standard deviation
    6. Find the z-score
    0. Quit program'''
    print(prompt)

    choice = input('Pick one: ')
    choices = {
        '1': find_mean,
        '2': find_median,
        '3': find_mode,
        '4': find_range,
        '5': standard_deviation,
        '6': z_score,
        '0': quit
    }

    if choice in choices:
        choices[choice]()
    else:
        print_failure()

Give some defaults, upper case - default value

print("[Y/n]") 
print("[y/N]") 

Use string.format instead of deprecated %. Replace

print("The range of %s and %s is %s" % (x, y, result))

with

print('The range of {} and {} is {}'.format(x, y, result))
\$\endgroup\$
  • \$\begingroup\$ As a side note, if you use a dictionary (which is a brilliant idea), you could use it to store the strings used in the prompt too so that the making from number to effect is effectively done in a single or place. \$\endgroup\$ – SylvainD Jun 18 '15 at 7:12
  • \$\begingroup\$ Or, as they're numbered, use a list: [('Quit program', quit), ('Find the mean', find_mean), ...]. \$\endgroup\$ – jonrsharpe Jun 18 '15 at 11:35
  • \$\begingroup\$ @jonrsharpe Are you kidding me? I'm not asking any questions. There are thousands of improvements and you suggestion is not exception. \$\endgroup\$ – outoftime Jun 18 '15 at 12:15
  • \$\begingroup\$ @outoftime what do you mean am I kidding?! I'm only making a suggestion, calm down. \$\endgroup\$ – jonrsharpe Jun 18 '15 at 12:22
  • 1
    \$\begingroup\$ This is better than if and elif but you are repeating yourself: the message should be generated from the dictionary. \$\endgroup\$ – Caridorc Jun 18 '15 at 12:38
7
\$\begingroup\$

Don't use recursion to redisplay a menu on invalid choices. With enough invalid input, the call stack will overflow. It's an ugly practice. Use a loop instead: repeat until user chooses to exit, print an error on invalid choices, call a function on valid choice. After exiting this loop, there will be no more statements to execute, so the program will exit naturally, without calling exit explicitly.

Use with for file operations:

with open(path) as fh:
    for line in fh:
        # ...

This way the file handle will be correctly closed when you exit the block.

Avoid sys.exit as much as possible. It's better to break out of the main program loop naturally, without explicitly exiting.

Avoid global variables. The load samples method could return the samples instead of seeing a global variable.

At the end of each operation you give the user the option to exit the program immediately. It would be better to control the program flow in the main loop, and have one common point of exit. It will reduce some repetition.

In the main menu, most choices lead to the execution of a method that contains the implementation of that action. Except when exiting, there is some code instead of a function call. It would be better to move that code to its own function.

\$\endgroup\$
  • \$\begingroup\$ I tried this, and it seems to work. with open("sample_list.txt") as f: for line in f: samples.append(line) samples = [float(x) for x in samples] Is that along the lines you're suggesting? \$\endgroup\$ – Branden M. Ardelean Jun 18 '15 at 5:30
  • \$\begingroup\$ More or less. But don't build a list of strings and transform to a list of floats: build a list of floats directly \$\endgroup\$ – janos Jun 18 '15 at 5:34
  • \$\begingroup\$ The samples are read from a text file, so I'm not sure what other (equally complicated) option there is at the moment. \$\endgroup\$ – Branden M. Ardelean Jun 18 '15 at 5:37
  • 2
    \$\begingroup\$ explicitly exciting lol \$\endgroup\$ – o0'. Jun 18 '15 at 8:48
  • 1
    \$\begingroup\$ When the user chooses quit, then break \$\endgroup\$ – janos Jun 18 '15 at 15:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.