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.

Github to clone if you want

#!/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

    def scan(self):


if __name__ == "__main__":
    master = Tk()

1 Answer 1



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.


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.