3
\$\begingroup\$

This is a program that I created using Python. Please help me optimize the code.

import time
import sys
import socket
from time import localtime, strftime

# start up system


def start():
    print("Welcome, please choose what to do")
    choice = input('register/login ')
    if choice == 'register':
        register()
    elif choice == 'login':
        username()
    elif choice == 'admin':
        username()
    else:
        problems()


def register():
    registeragain = False
    global user
    global passs
    user = input('Please register your username : ')
    passs = input('Please register your password : ')
    register = input('Do you want to register another account? Y/N : ')
    if register == 'Y':
        registeragain = True
        register()
    elif register == 'N':
        username()
    else:
        print("Error")
        problems()

# login system


def username():
    global admin
    admin = 'Max'
    userinputname = input('Please enter the username : ')
    if userinputname == admin or userinputname == user:
        password()
    elif userinputname != user:
        print('Wrong username, please try again')
        username()
    else:
        problems()


def password():
    x = 1
    global adminpass
    adminpass = 'Luo'
    userinputpass = input('Please enter the password : ')
    if userinputpass == adminpass:
        adminlogin()
    elif userinputpass == passs:
        loading()
    elif userinputpass != passs:
        print('Wrong password, please try again')
        x += 1
        if x > 2:
            exit

    else:
        problems()

# main menu


def loading():
    toolbar_width = 40
    # setup toolbar
    sys.stdout.write("[%s]" % (" " * toolbar_width))
    sys.stdout.flush()
    # return to start of line, after '['
    sys.stdout.write("\b" * (toolbar_width+1))

    for i in range(toolbar_width):
        time.sleep(0.1)  # do real work here
        # update the bar
        sys.stdout.write("-")
        sys.stdout.flush()

    sys.stdout.write("]\n")  # this ends the progress bar
    main()


def main():
    time.sleep(3)
    print("Welcome to the main screen")
    time.sleep(3)
    print("Menu: current time, ipchecker, quit")
    time.sleep(2)
    mainmenu = input('Enter: ')
    if mainmenu == 'current time':
        telldate()
    elif mainmenu == 'ipchecker':
        ipchecker()
    elif mainmenu == 'quit':
        leave()
    elif mainmenu == 'cal':
        cal()
    else:
        problems()

# solutions


def problems():
    print("We are truly sorry that you have experienced a problem, please contact us at [email protected]")
    option = input('Would you like to return to the main menu? Y/N ')
    if option == 'Y':
        main()
    else:
        exit()



# admin
def adminlogin():
    print("Welcome admin")
    mainadmin = input('Enter: ')
    if mainadmin == 'current time':
        telldate()
    elif mainadmin == 'ipchecker':
        ipchecker()
    elif mainadmin == 'quit':
        leave()
    elif mainadmin == 'cal':
        cal()
    else:
        problems()

# functions


def telldate():
    localtime = time.asctime(time.localtime(time.time()))
    print("Local current time :", localtime)
    main()


def ipchecker():
    hostname = socket.gethostname()
    IPAddr = socket.gethostbyname(hostname)
    print("Your Computer Name is:" + hostname)
    print("Your Computer IP Address is: " + IPAddr)
    main()

def cal():
    numx = int(input('Enter the first number: '))
    numy = int(input('Enter the second number: '))
    op = input('Enter the operation(+, -, *, /): ')
    if op == '+':
        sum = numx + numy
        print('Your sum is ' , sum)
        main()
    elif op == '-':
        diff = numx - numy
        print('The difference is ',diff)
        main()
    elif op == '*':
        mult = numx * numy
        print('The multiple is ' ,mult)
        main()
    elif op == '/':
        divd = numx / numy
        print('The result is ' ,divd)
        main()
    else:
        problems()

def leave():
    exit()

start()
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Some general notes:

  1. Global variables are usually a bad idea; state that can be accessed from everywhere is hard to read and debug because you have no idea what random function might modify it. It's extra difficult here because your globals are declared in different places.

  2. The standard way of handling errors in Python is to raise an exception that can be caught and handled.

  3. Long sequences of if/elif are usually an indication that your logic can either be simplified or split up.

Here's how I might suggest writing your start and register functions, using exceptions to handle errors, a lookup table to handle dispatching your different functions, and a single state object that collects all the random global variables:

from typing import NamedTuple

class LoginState(NamedTuple):
    admin: str
    adminpass: str
    user: str
    pass: str        

def start() -> None:
    state = LoginState("Max", "Luo", "", "")
    print("Welcome, please choose what to do")
    menu = {
        'register': register
        'login': username
        'admin': username
    }
    choice = input('register/login ')
    try:
        menu[choice](state)
    except:
        problems()

def register(state: LoginState) -> None:
    state.user = input('Please register your username : ')
    state.pass = input('Please register your password : ')
    menu = {
        "Y": register
        "N": username
    }
    choice = input('Do you want to register another account? Y/N : ')
    menu[choice](state)

Defining your state as a type means that it's easier to refactor -- for example, suppose you realize that your register logic overwrites the username and what you actually want to do is be able to have more than one! You could change user and pass into a collection (like a Dict, say), and use mypy to tell you all the places in your program that those attributes are used so you can make the appropriate updates. If they're defined as globals, it's a lot more difficult to make sure you've updated all the places where they're used.

The menu dict is IMO a little easier to expand (and read) than the big chain of if/elif, by providing a single obvious place where the menu options are defined.

Having problems() be an exception handler means that not only does it catch the case where the user inputs an invalid command (this will cause menu[choice] to raise an IndexError), but it will also catch any unhandled exceptions raised by your other functions, so those functions don't need to call problems() themselves; all they have to do is raise an exception and the control flow will end up right here. In the case of register() I didn't have to do anything with the error case, because if an exception is raised there I know that it will be caught in start().

Since the "present menu, make selection" pattern is probably going to be repeated a few times in this program, I might want to make that into a function as well:

def menu_prompt(
    menu: Dict[Text, Callable[[State], None]], 
    prompt: str,
    state: LoginState
) -> None:
    choice = input(prompt)
    menu[choice](state)

and now this:

    menu = {
        'register': register
        'login': username
        'admin': username
    }
    choice = input('register/login ')
    try:
        menu[choice](state)
    except:
        problems()

becomes:

try:
    menu_prompt({
        'register': register
        'login': username
        'admin': username
    }, 'register/login ', state)
except:
    problems()
\$\endgroup\$
2
  • \$\begingroup\$ and what else should I replace the global variables with? \$\endgroup\$
    – Zer0
    Nov 16, 2019 at 22:56
  • \$\begingroup\$ You don't need to replace them with more than one thing. :) Any particular piece of state should only live in one place as a general rule. \$\endgroup\$
    – Samwise
    Nov 16, 2019 at 23:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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