Register/Login and authentication through terminal

This is a registration and login program I made in Python that runs through the terminal. I am new to programming so I didn't have any actual use for this, I simply made it for practice. Please give me tips on minimizing code and making things shorter, point out any bad programming practices I used here and how I can change them and look out for them next time.

```
# Import modules
import time

# All accounts
users = {
"root": {
"password": "gucci-mane",
"group": "admin",
"mail": []
}
}

# Form validation
def validate(form):
if len(form) > 0:
return False
return True

# Login authorization
def loginauth(username, password):
if username in users:
if password == users[username]["password"]:
print("Login successful")
return True
return False

# Login
def login():
while True:
username = input("Username: ")
if not len(username) > 0:
print("Username can't be blank")
else:
break
while True:
password = input("Password: ")
if not len(password) > 0:
print("Password can't be blank")
else:
break

if loginauth(username, password):
return session(username)
else:
print("Invalid username or password")

# Register
def register():
while True:
username = input("New username: ")
if not len(username) > 0:
print("Username can't be blank")
continue
else:
break
while True:
password = input("New password: ")
if not len(password) > 0:
print("Password can't be blank")
continue
else:
break
print("Creating account...")
users[username] = {}
users[username]["password"] = password
users[username]["group"] = "user"
users[username]["mail"] = []
time.sleep(1)
print("Account has been created")

# Send mail
def sendmail(username):
w

In your validate function, instead of doing a conditional, returning True if is passes and False if it does not, just return the conditional that you are checking.

Here is what I mean:

return len(form) > 0

This will return True if the expression evaluates to True, and False if it evaluates to False. No conditional needed.

Or, if you wish to further simplify this function, just return the length of form. Python treats 0 as False so if the length is 0, it will act as False in a conditional.

At that point, it may not even be worth it as a function.

In your login function, you wrote this in your first if statement:

not len(username) > 0

Which looks almost exactly like your validate function, except for the not.

If you have the function validate, you might as well use it when you need it:

not validate(username)

You to be repeating something like this throughout your code:

if not len(username) > 0:
    print("Username can't be blank")
else:
    break

And the only thing that is really changing is this thing that can't be blank.

I recommend making a function so you can aren't repeating yourself as much.

Here is what I came up with

def cant_be_blank(form, name):
    if not len(form) > 0: # or, use `not validate`
        print(name + " can't be blank.")
        return False
    return True

Then, you can use the function like this:

if not cant_be_blank(username, "Username"):
    break

Add some documentation to your functions.

In your documentation, you should include things like what each parameter means (if there are any), what the return means (if there is any), and a short description of what the function does.

I wrote this for your loginauth function:

def loginauth(username, password):
     """
     * Confirms that the username exists and that the password is correct for that username
     * @param(str) username -- the username
     * @param(str) password -- the password
     * @return(bool) True -- if successful login
     * @return(bool) False -- if unsucessful logic (either username does not exist, or password is incorrect)
     """
    if username in users:
        if password == users[username]["password"]:
            print("Login successful")
            return True
    return False

Note the placement of the, as it is called, docstring.

On statements like this:

if not len(username) > 0:
    print("Username can't be blank")
    continue
else:
    break

I'm not sure how much of a difference this makes in efficiency (if it makes a difference in anything at all), but the else part is unnecessary.

If the conditional passes, continue will run and skip over everything else to go back to the top of the loop. If it does not run, it can just call down to break and exit the loop. No else needed.

So the above code snippet will become this:

if not len(username) > 0:
    print("Username can't be blank")
    continue
break # if the above conditional fails, execution will fall through to this

I recommend creating a class for users so information is more easily stored.

It doesn't have to be too complicated; just a simple storage of values:

class User:

    def __init__(self, username, password, group):
        self.username = username
        self.password = password
        self.group = group
        self.mail = []

Then, when inserting new Users into users, just do:

users[username] = User("john", "smith", "user")

In case in the future you make some mail parser, don't make it too complicated by storing the mail separated by strings ("Sender", "Subject", "Context"); store it in an object. Depending on what the user wrote, those strings can make messages really confusing.

Why not store mail in an object?

Here is what I came up with:

class Mail:

    def __init__(self, sender, subject, context):
        self.sender = sender.username # assuming you use the `User` class I wrote
        self.subject = subject
        self.context = context

    def to_string():
        print("Sender: %s\nSubject: %s\nContext: %s" % (self.sender, self.subject, self.context))

I threw in an extra to_string method to format the object's properties in a way that made sense to output.

This is not coding related, but I believe the correct word would be "content" rather than "context".

If you continue to work on this code, you are going to build up quite a big if/elif/else statement in your session function.

As an easier way to solve this problem, create a dictionary containing the name of the command and the function to call for that command.

Here is what I mean:

commands = {
    "logout": logout,
    "view mail": view_mail
    ...
}

Then, your session function can be reduced to:

if option in commands:
    return_code = commands[option]()
    if return_code == False:
        break
else:
    print(option + " is not an option")

Now, a command should

StackExchange Code Review Q#95499, answer score: 7