# Letting unbanned users comment

I am a beginner in python and I was wondering if the code for this program is nice and clean. The program works I was wondering how could i improve it. So this program asks for your 'username' and checks to see if your 'username' is in the 'banned users' list. If it is then it will 'log out' and won't let you comment. If your 'username' isn't in the 'banned uses' list it will let you comment and ask if you would like to let someone else to comment. If you enter 'yes' the 'while loop' will run again if there isn't then the program will print the username and comments. This is basically what the program does. How can I improve it?

# We define a function here.
if user_comments:  # If the dictionary 'user_comments' has at least one key value pair then execute this block of code.
for username, comment in user_comments.items(): # Define a for loop for each key value pair in the dictionary 'user_comments'.
print(f"\n\t{username}: {comment}.")  # Print each key value pair.

comment_prompt = "Comment: "
continue_prompt = "\nWould you like to let another user comment (yes/no)? "

banned_users = ['pete', 'jack', 'ali', 'henry', 'jason', 'emily'] # List of banned users.
user_comments = {} # Empty dictionary that we will store user input later on in the program.

account_active = True  # Set up a flag

while account_active: # Run as long as the 'account_active' flag remains 'True'.

if username.lower() in banned_users: # Cross reference the username with the banned_users list. If the username is in the banned_users list then execute the following code.
print(f"\nI'm sorry {username} but you are banned from using this site.") # Tell the user that they are banned.
print("Deactivating...") # Simulate logging out.

account_active = False # Set the flag to 'False'

else: # Run this code only if the user is not in the 'banned_users' list.
comment = input(comment_prompt) # Ask the user for their comment and store it in the variable 'comment'.

repeat = input(continue_prompt) # Ask the user if they want to repeat and store it in the variable 'repeat'.

if repeat == 'no': # If the variable 'repeat' has the value 'no' then execute the following code, otherwise run the while loop again.
account_active = False # Set the flag to 'False'

display_comments() # Call the 'display_comments()' function to print the key-value pairs in the dictionary.

• Usually you would put a summary of the problem or what the code does in the title. The desire to improve the code is implied by posting to this site. – spyr03 Aug 14 at 14:01
• spyr03 Thanks bro, I changed it. – snow_razer Aug 14 at 14:15

The code is pretty clear in what it does, good work.

# We define a function here.
def ...
...
... = {} # Empty dictionary
...
... = False # Set the flag to 'False'


Many of these comments are not all that useful. I can see by looking at the code that the flag is set to false, or that a function has been defined. Usually you would comment with the reason why a piece of code exists. If you removed a comment from beside some code, could you still work out what the code does? If so the comment is probably not useful.

def display_comments():
if user_comments:  # If the dictionary 'user_comments' has at least one key value pair then execute this block of code.
for username, comment in user_comments.items(): # Define a for loop for each key value pair in the dictionary 'user_comments'.
print(f"\n\t{username}: {comment}.")  # Print each key value pair.


This looks pretty good. It does what it says it does. Remove the inline comments and it is golden.

banned_users = ['pete', 'jack', ...
...


Extracting banned users to a list is good. An issue might arise here if a with an uppercase letter is is added to the banned list. He won't be prevented from commenting! In general there are two ways to avoid this problem

1. Do a case-insenstive compare. In a Java this would be done with username.equalsIgnoreCase(banned_user)
2. Normalize both strings and then compare. This is the recommended method in Python.

So I would suggest making sure every banned user has been lowercased

banned_users = ['pete', 'jack', ...
banned_users = [banned_user.lower() for banned_user in banned_users]


If you believe you'll ever need to deal with international usernames casefold may be of interest. It will normalize strings more aggressively than lower will.

One result of lowercasing the names is that it cuts down on the number of available usernames. If Ben Dor and B. Endor both try to sign up as BenDor and BEndor respectively, only one can get the name. Is that OK? Will this ever be a problem?

repeat = input(continue_prompt)
if repeat == 'no':


This is one place I would lowercase as an answer of "NO" or "No" clearly indicate the user is done.

user_comments = {}
...


As the comments are stored in a dictionary, every username will only have one comment, the latest one they've made. For instance if the chat was meant to be

A: where is the coal?
B: walk west for 2 mins then
B: north for 1 min


A: where is the coal?
B: north for 1 min


Is that intended? If not you can change to use a list containing the username and comment instead

comments = []
...

for username, comment in comments:

• The keyword to search is tuple unpacking. If first_pair = comments[0] you can unpack the pair with username, comment = first_pair. Since for x in y assigns to x each object in y, you can unpack in the loop header. Thus for pair in comments becomes for username, comment in comments. – spyr03 Aug 14 at 16:22