Can the following code be simplified? I think that the program that I have created is very excessive and quite slow. The program runs fine, but I would like to understand if this could actually be programmed better.

I have commented on every line of the code. Any kind of feedback on the program will be great, as I have only been programming for a couple of weeks now.

# Python 3.4.3. Using MacOS (Version 10.12.5)
# The programs purpose: A user must enter the correct username and password for a site called FaceSnap...
# The correct username is elmo and the correct password is blue.

count = 0 # Create a variable, to ensure the user has limited attempts at entering their correct username and password
count += 1 # The user has already had one attempt above, therefore count has been incremented by 1 already.

if count == 3: # Counter, to make sure the user only gets a limited number (3)of attempts
print("\nThree Username and Password Attempts used. Goodbye") # Lets the user know they have reached their limit
break # Leave the Loop and the whole program

elif userName == 'elmo' and password == 'blue': # The userName and password is equal to 'elmo' and 'blue', which is correct, they can enter FaceSnap!
print("Welcome! ") # Welcomes the User, the username and password is correct
break # Leave the loop and the whole program as the username and passowrd is correct

elif userName != 'elmo' and password != 'blue': # The userName and password is NOT equal to 'elmo' and 'blue', the user cannot enter FaceSnap
userName = input("\n\nUsername: ") # Requests the user to have another attempt at entering their correct username
password = input("Password: ") # Requests the user to have another attempt at entering their correct password
count += 1 # Increments the count by 1
continue # Continue, as the user hasn't managed to get their username and password correct yet

elif userName == 'elmo' and password != 'blue': # The userName is equal to 'elmo', but password is NOT equal to 'blue', the user cannot enter FaceSnap
userName = input("\n\nUsername: ") # Requests the user to have another attempt at entering their correct username
password = input("Password: ") # Requests the user to have another attempt at entering their correct password
count += 1 # increments the count by 1
continue # Continue, as the user hasn't managed to get their username and password correct yet

elif userName != 'elmo' and password == 'blue': # The userName is NOT equal to 'elmo', however password is equal to 'blue', the user cannot enter FaceSnap
userName = input("\n\nUsername: ") # Requests the user to have another attempt at entering their correct username
password = input("Password: ") # Requests the user to have another attempt at entering their correct password
count += 1 # Increments the count by 1
continue # Continue, as the user hasn't managed to get their username and password correct yet


A few points:

• You can simplify the if loop by using the else condition.

• Getting input should be at the beginning of the while loop, because it makes more logical sense to put it there. It's what happens every time you restart.

• You can use the simple condition of True for your while loop.

• You don't need the continue keyword, as it is at the end of your loop.

count = 0
while True:
count += 1
if count == 3:
#tells user bye
break #exit
else:
#let them in
break #they are in, exit loop
else:
#tell them it is wrong and have them retry, stay in loop

• Excellent, thank you for your time and points above. I will work on your above comments. Finally, I ran your code through python Idle and it was much more simple. Can you recommend a good IDE for Python please? – Greg May 28 '17 at 21:31
• It is better to write while count < 3: and remove if from the loop body – belkka Jul 6 '18 at 15:16

## Use hashes

Merely hashing is very weak, but this is rather easy to implement. Also note that md5 is prone to hash collision attacks and should not be used anymore. If your platform supports it, use SHA(3)-256.

The problem with this application is that if anybody gets to access the code, they can read the username and password in plaintext. You would be better off first hashing the passwords, then using hashlib to check if the input matches. I've written an improved version of your code below, to show what I mean:

from hashlib import md5
from getpass import getpass
import sys

print("Hello! Welcome to FaceSnap!")

attempts = 0
# These hashes have been generated earlier on.
# but for the sake of simplicity, we'll do it like this.

while True:
# Getpass will not echo input to the screen, so your password remains
# invisible
print()

if attempts == 3:
sys.exit("Too many failed attempts.")

else:
attempts += 1
else:
attempts += 1


## Use Scrypt

In reality, you wouldn't hash passwords to store them in a database, but use a dedicated key derivation function, such as Scrypt. The first cryptography library for Python that comes to mind is PyCrypto, but cryptography makes it significantly harder to shoot yourself in the foot.

• Firstly, Thank you for your reply. I do have two questions regarding the hashes in the variables check_username and check_password. How did you create the hashes to represent elmo (username) and blue (password)? Finally, how does the program know that the hashes are equal to elmo and blue? – Greg May 28 '17 at 18:38
• Hey Greg, I started a Python session and used from hashlib import md5 => md5(b"elmo").hexdigest() and md5(b"blue").hexdigest(). A hash is fixed-length digest of a message. In this case, a 'message' is a string of any length. This means you can hash one character, but also a whole book. Something to note is that hashing a string multiple times will always generate the same hash. You may want to read about hashing algorithms, it's quite interesting. Now on to your question, the program also imports md5, hashes the input and compares the hashes to the hashes it knows. Hope I helped! – Daniel May 28 '17 at 18:51
• Brilliant. I understand this now, I will definitely do some further reading on hashing algorithms! By the way, I meant this section of the code* when I mentioned variable. Thank you again! *check_username = '5945261a168e06a5b763cc5f4908b6b2' – Greg May 28 '17 at 19:47
• Hi @Coal_ I hope you are well. Apologies for an additional question. I have just noticed that the hashes are always producing the same encryption, for the same word. Therefore, couldn't a person who had gained access to the source easily find out that '5945261a168e06a5b763cc5f4908b6b2' is equal to 'elmo', for example? Thank you! – Greg May 29 '17 at 19:45
• G'day to you again :P don't worry about asking another question, I'm happy to help. Regarding your question, the idea of hashing is not to be able to reverse it. This means it isn't technically encrypting anything, because you cannot decrypt a given hash. A common attack, however, can be a rainbow-tables attack (exploit-db.com/docs/104.pdf). Simplified; attackers try hashing common passwords (1234, passw0rd, etc.) until a hash matches the one they found in the database. – Daniel May 30 '17 at 5:14

This will be a short one, but when you start you write count = 0, and then count += 1, where you could just as easily written count = 1 to start off with.

Or you can do this:

attempts=0
while attempts<3:
print('you are in!')
else:
attempts+=1
print('incorrect!')
if attempts==3:
print('too many attempts')


What this does is create a loop that asks you for a correct username and password, and checks if it is correct through an if-statement. Then, when the attempts have exceeded the maximum amount of allowed attempts, tells the user that.

• I made it shorter. Shouldn't that be what the asker wanted? Simplified? – John Hao May 28 '17 at 10:42
• Okay. Sorry. I understand what you are saying. – John Hao May 28 '17 at 12:13
• Hi, thank you for the attempt at answering this, however I have decided which answer was most suitable for my question. Thank you. – Greg May 30 '17 at 23:23

In terms of design, there are two main things you should take in consideration:

1. It is not good to have lot of if statements, especially in your case. You need to refactor your code in the name of the Anti-If Compaign.
2. You should accomplish the tasks through small units of code called functions.

In terms of security, there are also two main things to consider:

1. When an attacker targets your applications and types the right username, you inform him that the user exists, but only the password is wrong. This is bad in the security perspective because it increases the attack surface of your application. You can overcome this by displaying a more generic and less informative error message instead.
2. I do not know if your application is running on the Internet or offline. It is recommended that you hide the password which is being typed for the sake of reducing the attack surface too.

Given the above 4 observations, I want to provide you an improved version of your code through an MCVE where:

• I use getpass module to hide the password when it is typed.
• I use a predefined user credentials to simulate a more realistic situation (such as may be in your case you get the credentials from a database)
• I use a separate function to read the user credentials
• I use an other function which only purpose is to validate the user input.

These steps will lead us to using one if statement only:

#!/usr/bin/env python
# -*- coding: utf-8 -*-
import getpass

def get_user_credentials():

def check_user_credentials(user_credentials):

if __name__ == "__main__":
correct_credentials = check_user_credentials(user_credentials)
while not correct_credentials and login_attempts < 3:
print('\nWrong credentials, try again:')
correct_credentials = check_user_credentials(user_credentials)
if correct_credentials:
print('Welcome! Do whatever work you need here ...')
else:
print('Game over!')

• When I read this, I didn't think it implies that it's ok to store passwords in plain text, contrary to what Coal_ says. But, I find it painful to see such code, and I'm concerned that some inexperienced readers might be mislead by this. I'll remove the -1 if you rework that. – janos May 28 '17 at 16:38
• Hi, thank you for this. I have now made the answer clear at the top of the page. Thank you – Greg May 30 '17 at 23:24
• Then be honest with yourself by downvoting my answer @Coal_ – Billal Begueradj May 31 '17 at 16:43
• I have chosen not to downvote your answer because there was no need to do so at the time and there is even less reason to do so now. I explained why I didn't agree with your solution, instead of giving you a rather meaningless downvote. – Daniel May 31 '17 at 18:54
• Unbelievable hypocrisy: you accused me twice of encouraging to store passwords in plain text but you say it is meaningless to downvote? I am speechless (Also you mentioned hashing password in your answer which was posted previous to mine, so why do you ask people to include your answer and repeat it in theirs?) I have no respect for intellectually dishonest people. End of discussion @Coal_ – Billal Begueradj Jun 8 '17 at 4:03

## protected by Vogel612♦Dec 6 '18 at 13:16

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).