I am really new to programming, and I made this simple tkinter app in python. Please tell me what can be improved and how to use classes correctly in combination with tkinter.
#!/usr/bin/env python
# ---------------------------------
# @author: apoc
# @version: 0.1
# ---------------------------------
# importing
from tkinter import *
import csv
from random import randint
class LRG(object):
def __init__(self,master):
# variables
with open('data/champs.csv','r') as f:
reader = csv.reader(f)
self.champ_list = list(reader)
# layout
self.randombutton = Button(master,text='Random',command=self.scan)
self.infofield = Text(master,height=20,width=50)
# layout
self.randombutton.grid(row=0,column=1,sticky=E,pady=2,padx=5)
self.infofield.grid(row=1,columnspan=4,sticky=W,column=0,pady=4)
def scan(self):
self.infofield.delete('1.0',END)
self.infofield.insert(END,self.champ_list[
randint(0,len(self.champ_list))
])
if __name__ == "__main__":
master = Tk()
LRG(master)
master.title("LRG")
master.mainloop()
1 Answer 1
Standards
Follow the PEP8 coding standard. One violation that immediately is obvious is the lack of a space following a comma. For example, (row=0, column=1, sticky=E, pady=2, padx=5)
is much easier to read. Use pylint
(or similar) to check your code style against the recommended standard.
Import grouping
My personal preference is to keep import ...
statements together, and from ... import ...
statements together, not interleave them.
Private class members
"Private" members should be prefixed with an underscore. Such as self._champ_list
and def _scan(self):
. This doesn't actually make them private, but some tools will use the leading underscore as a key to skip generating documentation, not offer the member name in autocomplete, etc.
Remove useless members
self.randombutton
is only used in the constructor; it could just be a local variable, instead of a member.
Use correct comments
The # layout
comment above the Button
and Text
construction is misleading; it is not doing any layout.
Self documenting names
Use understandable names. I have no idea what LRG
is. Good variable names go a long way towards creating self-documenting code.
Use Doc-Strings
Add """doc-strings"""
for files, public classes, public members, and public functions. Using LRG
as a class name could be fine (in may be a common acronym at your company), but adding a doc-string for the class could spell out what the acronym stands for at least once.
Avoid hard-coded information
You have hard-coded reading the data/champs.csv
file into the constructor. Consider making it more flexible, such as passing in a filename with that as a default.
class LRG:
"""Literal Random Generator"""
def __init__(self, master, csv_filename="data/champs.csv"):
# ...etc...
Or, move the reading of the data outside of the class, and pass the champ_list
in as a constructor argument.