Login system created with Python

Question

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

Answer 1

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

• Zer0: and what else should I replace the global variables with?
• Samwise: 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.