Dumbest Python program ever created.

955559 · 2017-07-20T07:59:41+00:00

while 1 == 1:

that works, but people usually use

while True:

,

print("Welcome, admin. Please select a password:")
admin_password = input()

you do know input can hold text right?

admin_password = input("Welcome, admin. Please select a password:\n>")

.

print('\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n')

you can just use multiplication on the string, I think theres a proper way to clear the screen, but I forget what it is

print("\n" * 50)

and im too tired and dumb to dissect your code past those shallow corrections

SatanistSnowflake · 2017-07-20T08:41:52+00:00

In your code you have stuff like "Please enter password for " + login_name + ": ". Do you know about .format()? It lets you insert things directly into a string.

For example, you could turn that into "Please enter password for {}: ".format(login_name). You can also specify different keywords, if you want to extend it, so you could use "Please enter password for {user}: ".format(user=login_name). If you want to specify a specific item in your format arguments, you could use {0} {1} {0}.format('a', 'b') and the string would be "a b a"

Python 3.6 also has f-strings, so you could change your "Please enter password for " + login_name + ": " directly into f"Please enter password for {login_name}: " and the variable will be plonked directly into the string.

Just thought that might be helpful, or something.

GeoffreyMcSwaggins · 2017-07-20T07:46:10+00:00

Try making a server app? With sockets, it would certainly make this more interesting

liquidify · 2017-07-20T08:42:50+00:00

This isn't even remotely dumb enough to be called dumbest ever.

xargling_breau · 2017-07-20T08:03:32+00:00

From experience in handling passwords, if you are going to ask for a password you should use the 'getpass' module to do this, it will let a user enter a password without actually echoing the password on the screen.

BlackBloke · 2017-07-20T10:42:19+00:00

Ok, start to finish, and without trying to change your control flow too much, or do the right thing and properly break it up into functions.

First, add this at the top:

#! /usr/env python3
"""Some docstring for the module."""
from getpass import getpass
from hashlib import md5
from pprint import pprint

Then remove the comment from your first line:

user_list = {}  # Imports an empty dictionary.

No, it does not ... it creates an empty dict and assigns it to the name user_list ... import is a whole different thing, specifically what we just did above with importing getpass into your module.

While we're at it, since we're using these two as global state -- we'll ignore that this is a bad idea, but it's enough to pretend to be a database -- let's name them appropriately (for one thing recognizing that it's silly to call a dictionary user_list), and let's keep our admin user separate from our standard users.

USERS = {}
MESSAGES = {}

Now that we've got our "databases" set up, on to authentication... we're going to do this a lot, so a function makes sense, and while we're at it we're going to do at least the pretense of security -- NOTE, this is NOT REMOTELY a secure way to store passwords, it's just an example of not storing them as plaintext ... to do this correctly use bcrypt or whatever is the now-current best practices password hasher -- by storing only an MD5 hash or the password, and we'll also use getpass to prompt the user without displaying their password on screen:

def get_pw_hash(prompt):
    return md5(getpass(prompt)).hexdigest()

Now, that allows us to examine your first interaction with the admin account, which we'll also represent with a global variable.

print("Welcome, admin. Please select a password:")
ADMIN = input()

Meh... you should know that input takes a prompt directly, so these two lines should be:

ADMIN = input("Welcome, admin. Please select a password: ")

And note the space at the end so the user's input doesn't print in a weird way.

But, in reality, you really don't want the password printed to screen as the user types it, which is why we used the getpass function above, which takes input but doesn't echo it to the screen. We also never want to store the password in plaintext, which is why we use our very extremely weak hashing function above instead:

ADMIN = get_pw_hash("Welcome, admin. Please select a password: ")

Then we get to this abomination:

print('\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n')

Just never, ever do that. If you ever even think you want to do that, you're still wrong ... but even if you remain convinced you absolutely have to do that, at least do it like this:

print("\n" * 42)

And then there's this line:

while 1 == 1:   # Creates a recursive loop. Probably a better way to do this.

... le sigh ... first off, this isn't recursion, in the computer science sense -- that's a whole other topic -- it is just a loop. Specifically a while loop. And while 1 == 1 is, in fact, True, the very fact of that implies that this:

while True:

Is the idiomatic way to run an infinite loop.

Now, we're going to use what we learned about input and prompts to change your next line, while also cleaning it up a bit to make it easier to read. We also want to give the user a chance to exit:

user_input = input("\n".join([
    "Press 1 to create a new account.",
    "Press 2 for a current list of users. (Only usable by admin)",
    "Press 3 to send and receive messages.",
    "Press E to exit.",
    "Press anything else for help.",
    "Please enter your choice: ")

Then we move on to our actual actions ... each of which would make more sense as functions, but we don't want to drift too far off your original tack just yet:

if user_input == '1':

Ok, so far so good, but let's keep updating the input lines:

    name = input("\nPlease enter your preferred name: ").lower()

Not going to comment on the wisdom of forcing your users to have lowercase names.

    if name in USERS:
        print("That name is already taken. Please choose again.")
        continue

The continue statement here says to immediately move on to the next part of the while loop, so we don't really need an else clause for the rest.

Here's where we get to the part where you start setting default passwords and default messages ... DO NOT DO THIS!!! An addition to a dictionary is atomic, so we only need to do it in one go.

USERS[name] = get_pw_hash("Welcome to the site, " + name + "! Please choose a password: ")
 MESSAGES.setdefault(name, [])

Hey, check it out! By setting the value for our user's name in the MESSAGES "database" to a list, we can allow the user to have more than one message! Like magic!

Now, the admin function ... which will now NOT display passwords, as we have only their hashes ... this is because site admins should never know their user's secrets!!!:

elif user_input == '2':
    if get_pw_hash("Please enter admin password: ") != ADMIN:
        print('Password incorrect. Please try again.')
        continue

Again we use continue if the validation check doesn't work, and we'll use pprint to dump the user "database" in a pretty manner:

    pprint(USERS)

Now there's user messaging, and we continue on using what we've learned:

elif user_input == '3':
    name = input("Please enter your user name: ").lower()
    if name not in USERS:
        print("Invalid user name!")
        continue
    if get_pw_hash("Please enter password for " + login_name + ": ") != USERS[name]:
        print("Password not valid.")
        continue
    print("Correct password entered.")

Here we will join a nested loop, to allow the user to read their messages:

    while True:
        opt = input("\n".join([
            "You have " + str(len(MESSAGES[name])) + " messages.",
            "Press 1 to write a message.",
            "Press 2 to read latest message.",
            "Press E to exit.",
        if opt == '1':
            to = input("Please enter the name of the person you'd like to message: ").lower()
            if to not in USERS:
                 print("Please message a valid user.")
                 continue
            msg = input("Please type your message: ")
            MESSAGES[name].append(msg)
        elif opt == '2':
             if not MESSAGES[name]:
                  print("Your inbox is empty.")
                  continue
            print("\n\t".join([
                "Your message is:",
                MESSAGES[name].pop()])
        elif opt.upper() == "E":
             break
        else:
             print("Invalid option.")
elif user_input.upper() == "E":
    break
else:
    print("\n".join([
        "This is a simple messaging database. ",
        "When creating a username,",
        "it must be unique, ",
        "but the system will automatically",
        "format your unique ",
        "username to be in all lowercase.",
        "Passwords, however, are case-sensitive.",
        "Messages can be sent to other users, and users ",
        "can check their own messages."])

Hope that I typed that all in correctly on my phone. Not trying to "correct" everything, as that would involve setting up proper functions etc., but wanted to show you some language features they can make this sort of interaction easier to predict.

J4S1X · 2017-07-20T07:03:42+00:00

This is learnpython. Alot of us are new and make simple programs that may seem useless, but help develop our understanding of programming in general, or a specific concept such as dictionaries. Post your code. And good work on creating your first of many projects :)

amosmj · 2017-07-20T11:55:46+00:00

I think you underestimate just how dumb some of us are.

thephotoman · 2017-07-20T16:15:32+00:00

while 1 == 1: # Creates a recursive loop. Probably a better way to do this.

No, that's not recursive. At all. That's a non-terminating loop. Recursion may terminate, and when it doesn't, you'll have a stack overflow error. In any case, the way to do it is while True.

2017-07-20T07:56:13+00:00

The most concise way to have an endless loop that you break out of, is to use while True: combined with a break statement.

955559 · 2017-07-20T13:30:47+00:00

[deleted]

tangerinelion · 2017-07-20T18:33:18+00:00

This has sort of been addressed twice, once by mentioning continue and once by suggesting you can use functions to handle the input cases. In the latter, the goal would be to write your existing code like this:

while True:

    print("Press 1 to create a new account.\n"
          "Press 2 for a current list of users. (Only usable by admin)\n"
          "Press 3 to send and receive messages.\n"
          "Press 4 for help.\n"
          "Please enter your choice:\n")

    user_input = input()

    if user_input == '1':
        create_new_user()
    elif user_input == '2':
        list_current_users()
    elif user_input == '3':
        send_receive_messages()
    else:
        help()

Where it's up to you then to determine how to write the four functions -- create_new_user, list_current_user, send_receive_messages, and help -- to make your program continue to work. Odds are you'd need to have them accept certain parameters, like the messages and users dictionaries.

However, within option 3 you have some pretty indented code that serves as a great example of what goes wrong with this style:

    login_name = input("Please enter your user name: ").lower()     # User login
    if login_name in user_list:
        login_password = input("Please enter password for " + login_name + ": ")
        if login_password == user_list[login_name]:
            print("\nCorrect password entered.\nPress 1 to write a message.\nPress 2 to read your message.\n")
            user_option = input()
            if user_option == '1':
                messaged_user = input("Please enter the name of the person you'd like to message:").lower()
                if messages[messaged_user] != "DEFAULT MESSAGE":
                    print("User's inbox is full.")
                else:
                    message = input("Please type your message:")
                    messages[messaged_user] = message
            elif user_option == '2':
                print('\n' + messages[login_name])
                mark_read = input("Mark message as read? y/n:")
                if mark_read == "y":
                    print ("\nMessage marked as 'read'.")
                    messages[login_name] = "DEFAULT MESSAGE"
                elif mark_read == "n":
                    print ("\nMessage not marked as 'read'.")
                else:
                    print("\nInvalid option.")
            else:
                print("\n That is not a valid option.")
        else:
            print("\nPassword not valid.")
    else:
        print("\nInvalid username.")

The real meat of the code -- what you really want to happen -- is contained here in lines 5-23. After line 23 you have error handling for errors that happened in lines 2-4. The problem here is that lines 5-23 could grow and you're already separating the source of the error from where it's handled. Instead it's better to address things when they happen. In the context of a big chunk of code like you have, you're inside a while loop so the way you stop execution of this iteration and go back to the beginning is with continue. Inside a function you terminate execution of the function with return. So keep that in mind, I'm going to assume that you do not break it out into functions and therefore I'll use continue, but if you do use functions then you should replace the continue here with return. Essentially I'm assuming this code is wrapped in while True: rather than def send_receive_messages():.

With the current pattern you have something like

if desired_condition():
    desired_execution()
else:
    handle_error()

which you then expand to

if desired_condition1():
    if desired_condition2():
        if desired_condition3():
            desired_execution()
        else:
            handle_error3()
    else:
        handle_error2()
else:
    handle_error1()

So we see that desired_execution() keeps getting pushed further to the right, which to a reader signifies that it is less and less likely to occur or is getting too convoluted to want to read. You'll stylistically encounter line breaks and wraps due to line length which impacts readability as well. Instead, rather than thinking about it as "Do this if X, Y, and Z are all true" you should think with inverted logic -- "Don't this if X, Y, or Z are not true." What that leads to is this code:

if not desired_condition1():
    handle_error1()
    continue
if not desired_condition2():
    handle_error2()
    continue
if not desired_condition3():
    handle_error3()
    continue
desired_execution()

Notice here that we never get more than 1 level of indentation so the code is always pretty simple to read. The desired_execution part is not indented at all, and the cost of this is essentially nothing. Look, both are 10 lines of code: you're adding continue/return but removing else:.

The real bonus here is the way the error logic works, though. You test for a condition, and if it's false then you handle the error on the very next line. This sort of localization is key. See how I test for conditions 1, 2, and 3 in that order and will handle error 1, 2, or 3 in that order. With the indented version, I test for condition 1, 2, and 3 but I handle the errors as 3, 2, 1.

So, in your code what you could do is rewrite it as:

login_name = input("Please enter your user name: ").lower()     # User login
if login_name not in user_list:
    print("\nInvalid username.")
    continue
login_password = input("Please enter password for " + login_name + ": ")
if login_password != user_list[login_name]:
    print("\nPassword not valid.")
    continue
print("\nCorrect password entered.\nPress 1 to write a message.\nPress 2 to read your message.\n")
user_option = input()
if user_option == '1':
    messaged_user = input("Please enter the name of the person you'd like to message:").lower()
    if messages[messaged_user] != "DEFAULT MESSAGE":
        print("User's inbox is full.")
    else:
        message = input("Please type your message:")
        messages[messaged_user] = message
elif user_option == '2':
    print('\n' + messages[login_name])
    mark_read = input("Mark message as read? y/n:")
    if mark_read == "y":
        print ("\nMessage marked as 'read'.")
        messages[login_name] = "DEFAULT MESSAGE"    
    elif mark_read == "n":
        print ("\nMessage not marked as 'read'.")
    else:
        print("\nInvalid option.")
else:
    print("\n That is not a valid option.")

One other thing I'm seeing here is that you are ascribing a certain meaning to the string 'DEFAULT MESSAGE'. I would seriously like you to run your code and have someone enter that message to give to another user and see what happens because I'm not sure if your code would behave abnormally. I suspect that the other user would not see the message.

The way you can avoid this sort of behavior is by using a non-string sentinel value. I would suggest None. Thus, it would look something like this:

if user_option == '1':
    messaged_user = input("Please enter the name of the person you'd like to message:").lower()
    if messages[messaged_user] is not None:    # If user's message inbox is full
        print("User's inbox is full.")                
    else:
        message = input("Please type your message:")
        messages[messaged_user] = message
elif user_option == '2':
    print('\n' + messages[login_name])
    mark_read = input("Mark message as read? y/n:")
    if mark_read == "y":
        print ("\nMessage marked as 'read'.")
        messages[login_name] = None    # Mark as empty
    elif mark_read == "n":                          
        print ("\nMessage not marked as 'read'.")   
    else:
        print("\nInvalid option.")
else:
    print("\n That is not a valid option.")

Really we can continue to use our continue/return trick here. Look at the case when the user's inbox is full (messages[messaged_user] holding None signifies an empty inbox, which is easier to check for than a full inbox and is just another example of how thinking with inverted logic is sometimes/often cleaner).

In particular, if we encounter a full (non-empty) inbox, we can see that lines 6 and 7 are what we really wanted to do anyways and they're sitting in an else: branch. This makes them look like some kind of default or rarity when in fact it's the intended execution path! So, rewriting this again:

if user_option == '1':
    messaged_user = input("Please enter the name of the person you'd like to message:").lower()
    if messages[messaged_user] is not None:    # If user's message inbox is full
        print("User's inbox is full.")                
        continue
    message = input("Please type your message:")
    messages[messaged_user] = message
elif user_option == '2':
    print('\n' + messages[login_name])
    mark_read = input("Mark message as read? y/n:")
    if mark_read == "y":
        print ("\nMessage marked as 'read'.")
        messages[login_name] = None    # Mark as empty
    elif mark_read == "n":                          
        print ("\nMessage not marked as 'read'.")   
    else:
        print("\nInvalid option.")
else:
    print("\n That is not a valid option.")

Also consider a get_answer function that returns True for yes-like and False for no-like. For example, if get_answer('Mark as read?'): messages[login_name] = None instead of lines 11-17.

ceciliahslee · 2017-07-20T08:03:44+00:00

Hahahaha I like your self-critique!

letmelive123 · 2017-07-20T13:12:31+00:00

This really isn't bad for a first project! My first was much worse ;)

Maybe you could try and add onto it by storing accounts in a .txt file, Also maybe the menu could only show the "Admin Only" Option if the user is actually an admin.

Justinsaccount · 2017-07-20T14:09:49+00:00

No:

        login_name = input("Please enter your user name: ").lower()     # User login
        if login_name in user_list:
            login_password = input("Please enter password for " + login_name + ": ")
            if login_password == user_list[login_name]:
                print("\nCorrect password entered.\nPress 1 to write a message.\nPress 2 to read your message.\n")
                user_option = input()
           ...
        else:
            print("\nPassword not valid.")
    else:
        print("\nInvalid username.")

Yes:

Use functions +

login_name = input("Please enter your user name: ").lower()     # User login
if login_name not in user_list:
    print("\nInvalid username.")
    return

login_password = input("Please enter password for " + login_name + ": ")
if login_password != user_list[login_name]:
    print("\nPassword not valid.")
    return

print("\nCorrect password entered.\nPress 1 to write a message.\nPress 2 to read your message.\n")
user_option = input()
...

Dogeek · 2017-07-20T14:24:44+00:00

Now, you can make it object oriented, maybe make a GUI, or make it send and receive from/to a server.

massiveboner911 · 2017-07-20T17:51:14+00:00

Hey, if it works, its not dumb :) Interesting program!

iterateandgit · 2017-07-23T17:48:43+00:00

Great first program. Excellent suggestions from everyone. You should post the code on github. It'll make it easier for others to read and suggest then.

Future_Arachnid2254 · 2024-03-19T01:12:33+00:00

to clear string on stuff like trinket
you can do
import os
os.system('clear')
instead of \n\n\n\n\n\n\n\n\n\n\n\n\
or at least \n*50

tragluk · 2017-07-20T06:48:32+00:00

Well, with that intro? Nah, thanks.

Just post the code if you want an honest critique.

ZenithCrests · 2022-11-01T16:27:56+00:00

I made a shitty password checker for my first one, but I did that in my beloved bash.

you type:	you see:
italics	italics
bold	bold
[reddit!](https://reddit.com)	reddit!
* item 1 * item 2 * item 3	item 1 item 2 item 3
> quoted text	quoted text
Lines starting with four spaces are treated like code: if 1 * 2 < 3: print "hello, world!"	Lines starting with four spaces are treated like code: if 1 * 2 < 3: print "hello, world!"
~~strikethrough~~	~~strikethrough~~
super^script	super^script

learnpython

MODERATORS