Background
I started learning Python about 3 months ago. I feel like I learned a lot, but I would like to receive some advice from the pros and some feedback about my code. So I wrote a small GUI app. I would like to know if it's efficient, readable, fits the PEP8 rules, and if you would do it in a different way.
Code Purpose
A Tkinter application that is simply a login form. You can choose to count the number of times you've logged in as a specific user.
CSV File Example
Shaked, 123456, 0
Ellen, pass123, 0
Code
'''
A Python Exercise: Reading CSV File & Tkinter Implementation
Including the subjects: Grid Layout, Tk Variables and Binding
'''
import Tkinter as tk
import csv
CSV_FILE = "users.csv"
# Functions
def popup(title, msg):
'''Open popup window with title and msg'''
w = tk.Toplevel(root)
w.title(title)
w.minsize(200, 200)
tk.Label(w, text=msg).pack()
tk.Button(w, text="Close", command=w.destroy).pack(pady=10)
w.bind("<Return>", lambda f: w.destroy())
def read_from_file():
'''Read csv file and return a list like: [[username, password, count]]'''
try:
with open(CSV_FILE, 'rb') as f:
users = []
reader = csv.reader(f)
for row in reader:
row[2] = int(row[2]) # Make the count an integer so it can increase later
users.append(row)
return users
except IOError:
popup("Error", "File not found!")
def write_to_file(users):
'''Get a list of all users and write it to the csv file'''
with open(CSV_FILE, 'wb') as f:
writer = csv.writer(f)
for row in users:
row[2] = str(row[2])
writer.writerows(users)
class App(tk.Frame):
def __init__(self, master=None):
tk.Frame.__init__(self, master)
self.master = master
self.pack(anchor="w")
# Labels
tk.Label(self, text="Username:").grid(sticky="w")
tk.Label(self, text="Password:").grid(row=1, sticky="e")
# Entries
self.n_string = tk.StringVar()
self.p_string = tk.StringVar()
name = tk.Entry(self, textvariable=self.n_string, width=30)
passw = tk.Entry(self, textvariable=self.p_string, show='*', width=20)
name.grid(row=0, column=1, sticky="we")
passw.grid(row=1, column=1, sticky="we")
# Check Button
self.check_var = tk.BooleanVar()
check = tk.Checkbutton(self, text="Count Login attempts", variable=self.check_var)
check.grid(row=2, column=1, sticky="w")
# Login Button
login = tk.Button(self, text="Login", command=self.login)
login.grid(row=2, column=1, sticky="e")
# Binding
self.master.bind("<Return>", self.login)
def login(self, event=None):
'''Login attempt'''
users = read_from_file()
if not users:
return
for row in users:
if row[0] == self.n_string.get() and row[1] == self.p_string.get():
msg = "Welcome %s!\n\nYour login count is %d" % (row[0], row[2])
popup("Welcome", msg)
if self.check_var.get(): # Check if "Count Login attempts" button is checked
row[2] += 1 # add +1 login count
write_to_file(users)
break
elif row[0] == self.n_string.get():
print "Password incorrect."
break
else: # If there isn't a match of the username
print "User name incorrect!"
# GUI settings
root = tk.Tk()
app = App(root)
root.title("Login Form")
root.minsize(200, 200)
# Initalize GUI
root.mainloop()
-
\$\begingroup\$ Weird.. It does work for me. Did you create a Windows Comma Seperated file? (when using mac). The script doesn't create a csv file by itself. \$\endgroup\$Shaked Shachar– Shaked Shachar2017年10月13日 17:25:20 +00:00Commented Oct 13, 2017 at 17:25
2 Answers 2
Documentation
While it is good that you added a docstring at the top of the code, it does not convey much useful information. It should describe the purpose of the code and include details about the CSV file. For example:
"""
Tkinter application for a login form.
It reads and writes a CSV file containing user names and passwords.
The CSV file is named "users.csv" and is in the current working directory.
The CSV file has this format...
"""
The PEP 8 style guide recommends adding docstrings for classes.
Portability
I realize this question was posted many years ago when Python version 2.x
was prevalent, but now that it is deprecated, consider porting to 3.x.
It may be as simple as changing these 2 print
lines:
print "Password incorrect."
print "User name incorrect!"
to:
print("Password incorrect.")
print("User name incorrect!")
I get an error on the following line:
import Tkinter as tk
The error goes away when I change Tkinter
to tkinter
:
import tkinter as tk
Perhaps your version of Python or your operating system is more forgiving than mine.
Error diagnostics
It is good that you check for the file's existence and display an error message in the GUI:
popup("Error", "File not found!")
It would be helpful to the user to also show the file name:
popup("Error", f"File {CSV_FILE} not found!")
It's generally bad practice to store plaintext passwords. This is because if the (in this case) CSV is leaked, an attacker has knowledge of every username password combination. Instead you should store a cryptographic hash of the password using something like hashlib.
Then, when checking if the password is valid, you compare the hash of the input with what is stored in the CSV. If an attacker obtains the CSV they have the hash, but it is important to note that the hash of the hash != hash, so they can't immediately use the hashed password that is written in the CSV.