3
\$\begingroup\$

I am writing a lesson-based program to help teach friends and family the basics of Arabic. The program consists of different sections (letters, numbers, personnel pronouns, etc.) and each of those sections is in turn broken into different lessons. Each section has its own .py file. All of those files are then imported into the "Master" .py file which then displays the options available to the user.

Though I've been programming for ~2 years now, it's all been self-taught/internet help based. I've just recently started experimenting with classes (I know...).

What I'm mainly looking for is help refactoring my code and using it as an opportunity to improve my coding style. What I have right now works - it does what I want it to do. But, I'm sure it could be improved dramatically and my hope is that by learning how to improve this code I can become a better coder in general.

The code below is 350 lines long, which I know is a lot. But I would be very appreciative if folks would take some time and point out anything they think is done incorrectly or could be improved. I've done my best to keep it to PEP 8 but I'm sure I've missed a few things.

import sys
import os
import tkinter
from tkinter import *
from tkinter import messagebox
from tkinter import ttk
from operator import itemgetter

import Letters_Listening_Test
import Numbers_1_Intro_1, Numbers_1_Intro_2
import Numbers_License_Plate_Decoder
import Lesson_1_Part_1
import Personal_Pronouns_Intro_1, Personal_Pronouns_Intro_2
import Greetings_Intro_1, Greetings_Intro_2, Greetings_Intro_3
import Greetings_Conversation_1, Greetings_Conversation_2

class Initialize_Screen:
    def __init__(self, cfn, img):
        self.CurrentFrameNumber = cfn
        self.image = img

        for i in range(0, NoF):  #This is for question frames
            frames[i].grid_remove()

        cfn.grid(column = 0, row = 0, sticky = (tkinter.W, tkinter.E))    
        for i in range(0, 4):
            cfn.grid_columnconfigure(i, weight=1)
            cfn.grid_rowconfigure(2, weight = 1)
            cfn.grid_rowconfigure(5, weight = 1)

        tempimage = MenuBackgroundImages + str(img) +(".png")
        backgroundImage = PhotoImage(file = (tempimage))
        backgroundImage_Label = Label(cfn, image = backgroundImage)
        backgroundImage_Label.place(x = 0, y = 0, relwidth = 1, relheight = 1)
        backgroundImage_Label.tempimage = backgroundImage

    def home_button(self):
        home_button = Button(
            self.CurrentFrameNumber, text = "Home",
            font = ("Helvetica", 25), command = initialize_main_menue)
        home_button.grid(column=0, row = 0, sticky = (tkinter.W, tkinter.E))

    def profile(self):
        print("Still in progress")

    def profile_button(self):
        profile_button = Button(
            self.CurrentFrameNumber, text = "     Profile     ",
            font = ("Helvetica", 35), command = lambda: opening_screen.profile())
        profile_button.grid(column = 1, columnspan = 2, row = 3)

    def main_program_button(self):
        main_program_button = Button(
            self.CurrentFrameNumber, text = "LibLib Arriby",
            font = ("Helvetica", 35), command = initialize_lesson_menue)
        main_program_button.grid(column = 1, columnspan = 2, row = 4)

    def back_to_main_program(self):
        back_to_main_program_button = Button(
            self.CurrentFrameNumber, text = "Back",
            font = ("Helvetica", 25), command = initialize_lesson_menue)
        back_to_main_program_button.grid(column = 0, row = 0)

class Side_Frame:
    def __init__(self, current_frame):
        self.CurrentFrameNumber = current_frame
        sideframe=tkinter.Frame(self.CurrentFrameNumber,
                                width = 100, height = 100, bg = "gray")
        current_frame['borderwidth'] = 2
        current_frame['relief'] = 'sunken'
        current_frame.grid_propagate(True) #Turns off autoshrink of Frame widget
        current_frame.grid_rowconfigure(0, weight = 1)
        current_frame.grid_columnconfigure(1, weight = 1)
        current_frame.grid(column= 1, row = 1)

def initialize_main_menue():
    global opening_screen
    opening_screen = Initialize_Screen(frames[0], 1)
    opening_screen.profile_button()
    opening_screen.main_program_button()

def initialize_lesson_menue():
    lesson_menue = Initialize_Screen (frames[1], 2)
    lesson_menue.home_button()
    lessons()   

def lessons():
    lessonframe=tkinter.Frame(frames[1], bg = "gray",
                              borderwidth = 2, relief = "sunken")
    lessonframe.grid(column = 1, row = 1)    
    lessonframe.grid_rowconfigure(0, weight = 1)
    lessonframe.grid_rowconfigure(1, weight = 1)

    current_frame = lessonframe

##############################################################################
## The items below list out the titles of all of the different lesson plans ##
    letters_button = Button(
        current_frame, text = "Letters",
        font=("Helvetica", 25), command = letters)

    numbers_button = Button(
        current_frame, text = "Numbers",
        font=("Helvetica", 25), command = Numbers)

    lesson_1_button = Button(
        current_frame, text = "Lesson 1",
        font=("Helvetica", 25), command = lesson_1)

    letters_button.grid(column = 0, columnspan = 1, row=1,
                        sticky = (tkinter.W, tkinter.E))
    numbers_button.grid(column = 0, columnspan = 1, row=2,
                        sticky = (tkinter.W, tkinter.E))
    lesson_1_button.grid(column = 0, columnspan = 1, row = 3,
                        sticky = (tkinter.W, tkinter.E))  

def letters():
    letter_screen = Initialize_Screen(frames[2], 3)
    letter_screen.back_to_main_program()

    current_frame=tkinter.Frame(frames[2], width = 100, height = 100, bg = "gray")
    side_screen = Side_Frame(current_frame)

    def all_buttons_forget():
        letters_2_button.config(relief = RAISED, bg = "gray95")
        letters_3_button.config(relief = RAISED, bg = "gray95")
        letters_4_button.config(relief = RAISED, bg = "gray95")
        letters_1_button.config(relief = RAISED, bg = "gray95")

    class Letters:
        def __init__(self, LN):
            self.Lesson_Number = LN

            intro_1_button = Button(
                current_frame, text = "Letters "+str(LN)+": Introduction Part 1",
                font = ("Helvetica", 25), command = lambda: intro_1(LN))
            intro_1_button.grid(column = 1, columnspan = 2, row = 1)
            intro_1_button.config(bg = "gold")            

            intro_2_button = Button(
                current_frame, text = "Letters "+str(LN)+": Introduction Part 2",
                font = ("Helvetica", 25), command = lambda: intro_2(LN))
            intro_2_button.grid(column = 1, columnspan = 2, row = 2)
            intro_2_button.config(bg = "gold")

            quiz_button = Button(
                current_frame, text = "Letters "+str(LN)+": Quiz",
                font = ("Helvetica", 25), command = lambda: quiz(LN))
            quiz_button.grid(column = 1, columnspan = 2, row = 3)
            quiz_button.config(bg = "gold")

    def intro_1(LN):
            module = __import__("Letters_" + str(LN)+ "_Intro_1")
            func = getattr(module, "letters_" + str(LN) + "_intro_1")
            func()

    def intro_2(LN):
            module = __import__("Letters_" + str(LN)+ "_Intro_2")
            func = getattr(module, "letters_" + str(LN) + "_intro_2")
            func()

    def quiz(LN):
            module = __import__("Letters_" + str(LN)+ "_Quiz")
            func = getattr(module, "letters_" + str(LN) + "_quiz")
            func()

    def letters_1():
        all_buttons_forget()
        letters = Letters(1)
        letters_1_button.config(relief = SUNKEN, bg = "gold")

    def letters_2():
        all_buttons_forget()
        letters = Letters(2)
        letters_2_button.config(relief = SUNKEN, bg = "gold")

    def letters_3():
        all_buttons_forget()
        letters = Letters(3)
        letters_3_button.config(relief = SUNKEN, bg = "gold")

    def letters_4():
        all_buttons_forget()
        letters = Letters(4)
        letters_4_button.config(relief = SUNKEN, bg = "gold")

    def about_letters():
        result = messagebox.showinfo("About this Section",
"The arabic alphabet consists of 28 letters which have been divided into four sections for ease of learning. Each section introduces two sets of letters and then provides a quiz on those letters. The section on letters is then concluded with a listening comphrension and spelling test.") 

    def additional_resources():
        imageroot = Root_File_Name + "Lessons\\Letters\\AdditionalResources\\"
        image = imageroot + "Arabic_Alaphabet.png"
        photo = PhotoImage(file = image)

        canvas = Canvas(tkinter.Toplevel(), height = 566, width = 850)
        canvas.grid(column=0, row=0)

        alphabet_photo = canvas.create_image(0, 0, image = photo, anchor = NW)
        alphabet_photo.image = photo

    about_button = Button(
        current_frame, text = "About",
        font=("Helvetica", 25), command = about_letters)
    about_button.grid(column = 1, columnspan = 1, row = 0,
                          sticky = (tkinter.W, tkinter.E))

    letters_1_button = Button(
        current_frame, text = "Letters 1",
        font=("Helvetica", 25), command = letters_1)
    letters_1_button.grid(column = 0, columnspan = 1, row = 1,
                          sticky = (tkinter.W, tkinter.E))

    letters_2_button = Button(
        current_frame, text = "Letters 2",
        font=("Helvetica", 25), command = letters_2)
    letters_2_button.grid(column = 0, columnspan = 1, row = 2,
                          sticky = (tkinter.W, tkinter.E))

    letters_3_button = Button(
        current_frame, text = "Letters 3",
        font=("Helvetica", 25), command = letters_3)
    letters_3_button.grid(column = 0, columnspan = 1, row = 3,
                          sticky = (tkinter.W, tkinter.E))

    letters_4_button = Button(
        current_frame, text = "Letters 4",
        font=("Helvetica", 25), command = letters_4)
    letters_4_button.grid(column = 0, columnspan = 1, row = 4,
                          sticky = (tkinter.W, tkinter.E))  

    listening_test_button = Button(
        current_frame, text = "Listening Test",
        font = ("Helvetica", 25), command = Letters_Listening_Test.letters_listening_test)
    listening_test_button.grid(column = 0, columnspan = 1, row = 5,
                          sticky = (tkinter.W, tkinter.E))

    additional_resources_button = Button(
        current_frame, text = "Additional Resources",
        font = ("Helvetica", 25), command = additional_resources)
    additional_resources_button.grid(column = 1, columnspan = 2, row = 5,
                          sticky = (tkinter.W, tkinter.E))

def Numbers():
    number_screen = Initialize_Screen(frames[3], 4)
    number_screen.back_to_main_program()

    current_frame=tkinter.Frame(frames[3], width = 100, height = 100, bg = "gray")
    side_screen = Side_Frame(current_frame)

    def all_buttons_forget():
        numbers_1_button.config(relief = RAISED, bg = "gray95")
        license_plate_decoder_button.config(relief = RAISED, bg = "gray95")

    def numbers_1():
        all_buttons_forget()
        numbers_1_button.config(relief = SUNKEN, bg = "gold")

        numbers_1_intro_1_button = Button(
            current_frame, text = "Intro to Numbers: Part 1",
            font = ("Helvetica", 25), command = Numbers_1_Intro_1.numbers_1_intro_1)
        numbers_1_intro_1_button.grid(column = 1, columnspan = 1, row = 1)
        numbers_1_intro_1_button.config(bg = "gold")

        numbers_1_intro_2_button = Button(
            current_frame, text = "Intro to Numbers: Part 2",
            font = ("Helvetica", 25), command = Numbers_1_Intro_2.numbers_1_intro_2)
        numbers_1_intro_2_button.grid(column = 1, columnspan = 1, row = 2)
        numbers_1_intro_2_button.config(bg = "gold")

    def about_numbers():
        result = messagebox.showinfo("About this Section",
"Though arabic letters are written right to left, arabic numbers are still written left to right. In this numbers section, the individual arabic numbers are introduced in two sets: 0-4, and 5-9.") 

    about_button = Button(
        current_frame, text = "About",
        font=("Helvetica", 25), command = about_numbers)
    about_button.grid(column = 1, columnspan = 1, row = 0,
                          sticky = (tkinter.W, tkinter.E))

    numbers_1_button = Button(
        current_frame, text = "Intro to Numbers",
        font=("Helvetica", 25), command = numbers_1)
    numbers_1_button.grid(column = 0, columnspan = 1, row = 1,
                          sticky = (tkinter.W, tkinter.E))
    numbers_1_button.config(highlightcolor = "gold")

    license_plate_decoder_button = Button(
        current_frame, text = "License Plate Decoder",
        font=("Helvetica", 25), command = Numbers_License_Plate_Decoder.license_plate_game)
    license_plate_decoder_button.grid(column = 0, columnspan = 1, row = 2,
                          sticky = (tkinter.W, tkinter.E))
    license_plate_decoder_button.config(highlightcolor = "gold")

def lesson_1():
    lesson_screen = Initialize_Screen(frames[4], 5)
    lesson_screen.back_to_main_program()

    current_frame=tkinter.Frame(frames[4], width = 100, height = 100, bg = "gray")
    side_screen = Side_Frame(current_frame)

    def all_buttons_forget():
        print("In progress")

    def about_letters():
        result = messagebox.showinfo("About this Section",
"Lesson 1 focuses on learning personel pronouns.") 

    about_button = Button(
        current_frame, text = "About",
        font=("Helvetica", 25), command = about_letters)
    about_button.grid(column = 1, columnspan = 1, row = 0,
                          sticky = (tkinter.W, tkinter.E))

    lesson_1_button = Button(
        current_frame, text = "Lesson 1",
        font=("Helvetica", 25), command = Lesson_1_Part_1.lesson_1_part_1)
    lesson_1_button.grid(column = 0, columnspan = 1, row = 2,
                          sticky = (tkinter.W, tkinter.E))
    lesson_1_button.config(highlightcolor = "gold")

#################################################################3
Root_File_Name = "C:\\LearningArabic\\LiblibArriby\\"
ImagePath = Root_File_Name + "Icon\\"
MenuBackgroundImages = ImagePath + "menue_screen_images\\"
icon_tiles = ImagePath + "menue_screen_images\\"

master = Tk()
master.title("LibLib Arriby") # Label the root GUI window.
##master.geometry('700x900+0+0')
##master.attributes("-fullscreen", True) #This would make it full screen

window_width = master.winfo_screenwidth()-100      # Define window size
window_heigth = (master.winfo_screenheight())-100

NoF = 6 #This is the number of frames
frames = []  # This includes frames for all questions
for i in range(0, NoF):
    frame=tkinter.Frame(
        master, width=window_width,
        height=window_heigth, borderwidth = 2,
        relief = "sunken")
    frame.grid(column = 0, row = 0, sticky = (tkinter.W, tkinter.E))
  #  frame.grid_remove()
    frame.grid_propagate(False) #Turns off  autoshrinking of Frame widget
    frames.append(frame)

initialize_main_menue()

master.mainloop()

Here are some photos of what it currently looks like. The photos are all taken here in Egypt where I live.

Main Screen Lesson Screen Letter Screen

\$\endgroup\$
3
\$\begingroup\$

Took me a long time to implement all the changes I made, so I hope you find this helpful!

  • Docstrings: You should include a docstring at the beginning of every method/class/module you write. This will allow documentation to identify what your code is supposed to do.
  • Unused Imports: You had a total of four unused imports. Having these in your code and confuse you and other people reading your code, as they/you try to find where you use them. Removing them can improve the readability of your code.
  • Parameter Spacing: There should not be any spacing between the parameter name, the =, and the value being passed. PEP-8 has rules about whitespace in expressions and statements.
  • Multiline Parameters: Should you have to have multiple lines to pass in parameters, each parameter gets its own line. The indentation should also be four spaces after the beginning of the line that the method starts on. That may have been confusing; look at the improved code.
  • Unused loop variables: When you want to loop through something, or do something a number of times without using the variable created, use an _. This makes it clear that the variable used in the loop should be ignored, since you don't intend on using it.
  • Variable Naming: Variables in python should be in snake_case, not Upper_Snake_Case or camelCase. Classes in python should be PascalCase.
  • String Formatting: You have strings that require you to use ... + str(...) + .... This is a big red flag to me. You should use f"" to directly include your variables into your strings, without having to cast them as other types (str(), int(), etc).
  • Global Variables: It is almost never a good idea to use global variables, in python or any programming language. This list does a very good job of explaining the negatives of using global variables, while also providing some instances where it might be okay. In general, though, it's not recommended.
  • Unneeded lambdas: You don't need to create a lambda to call a function.
  • Indentation: PEP-8 has rules about indentation. Specifically, all indentation should be four spaces. A few spots in your code you use eight. Wrongly indented code could possibly mess with the entire scope of your program.
  • Unneeded Variable Assignment: When you call mesagebox.showinfo(...), you assign it to a variable. This is not needed, since you don't use that variable.

Updated Code

"""
Module Docstring:
A description about this program goes here
"""

import tkinter
from tkinter import *
from tkinter import messagebox

import Letters_Listening_Test
import Numbers_1_Intro_1, Numbers_1_Intro_2
import Numbers_License_Plate_Decoder
import Lesson_1_Part_1
import Personal_Pronouns_Intro_1, Personal_Pronouns_Intro_2
import Greetings_Intro_1, Greetings_Intro_2, Greetings_Intro_3
import Greetings_Conversation_1, Greetings_Conversation_2

class InitializeScreen:
    """
    Class for setting up the screen
    """
    def __init__(self, cfn, img):
        self.current_frame_number = cfn
        self.image = img
        self.profile = "Still in progress"

        for index in range(0, NUMBER_OF_FRAMES):  #This is for question frames
            FRAMES[index].grid_remove()

        cfn.grid(column=0, row=0, sticky=(tkinter.W, tkinter.E))
        for num in range(0, 4):
            cfn.grid_columnconfigure(num, weight=1)
            cfn.grid_rowconfigure(2, weight=1)
            cfn.grid_rowconfigure(5, weight=1)

        temp_image = f"{MENU_BACKROUND_IMAGES}{str(img)}.png"
        background_image = PhotoImage(file=(temp_image))
        background_image_label = Label(cfn, image=background_image)
        background_image_label.place(x=0, y=0, relwidth=1, relheight=1)
        background_image_label.tempimage = background_image

    def home_button(self):
        """
        Builds the home button
        """
        home_button = Button(
            self.current_frame_number,
            text="Home",
            font=("Helvetica", 25),
            command=initialize_main_menue
        )
        home_button.grid(column=0, row=0, sticky=(tkinter.W, tkinter.E))

    def profile_button(self):
        """
        Buids the profile button
        """
        profile_button = Button(
            self.current_frame_number,
            text="     Profile     ",
            font=("Helvetica", 35),
            command=opening_screen.profile
        )
        profile_button.grid(column=1, columnspan=2, row=3)

    def main_program_button(self):
        """
        Builds the main program button
        """
        main_program_button = Button(
            self.current_frame_number,
            text="LibLib Arriby",
            font=("Helvetica", 35),
            command=initialize_lesson_menue
        )
        main_program_button.grid(column=1, columnspan=2, row=4)

    def back_to_main_program(self):
        """
        Builds the button that takes you back to the main program
        """
        back_to_main_program_button = Button(
            self.current_frame_number,
            text="Back",
            font=("Helvetica", 25),
            command=initialize_lesson_menue)
        back_to_main_program_button.grid(column=0, row=0)

class SideFrame:
    """
    Class that builds a side frame on creation
    """
    def __init__(self, current_frame):
        self.current_frame_number = current_frame
        current_frame['borderwidth'] = 2
        current_frame['relief'] = 'sunken'
        current_frame.grid_propagate(True) #Turns off autoshrink of Frame widget
        current_frame.grid_rowconfigure(0, weight=1)
        current_frame.grid_columnconfigure(1, weight=1)
        current_frame.grid(column=1, row=1)

def initialize_main_menue():
    """
    Initializes the main menu
    """
    opening_screen = InitializeScreen(FRAMES[0], 1)
    opening_screen.profile_button()
    opening_screen.main_program_button()

def initialize_lesson_menue():
    """
    Initializes the lesson menu
    """
    lesson_menue = InitializeScreen(FRAMES[1], 2)
    lesson_menue.home_button()
    lessons()

def lessons():
    """
    Creates the lesson frame, and the corresponding buttons
    """
    lessonframe = tkinter.Frame(
        FRAMES[1],
        bg="gray",
        borderwidth=2,
        relief="sunken"
    )
    lessonframe.grid(column=1, row=1)
    lessonframe.grid_rowconfigure(0, weight=1)
    lessonframe.grid_rowconfigure(1, weight=1)

    current_frame = lessonframe

##############################################################################
## The items below list out the titles of all of the different lesson plans ##
##############################################################################
    letters_button = Button(
        current_frame,
        text="Letters",
        font=("Helvetica", 25),
        command=letters
    )

    numbers_button = Button(
        current_frame,
        text="Numbers",
        font=("Helvetica", 25),
        command=Numbers
    )

    lesson_1_button = Button(
        current_frame,
        text="Lesson 1",
        font=("Helvetica", 25),
        command=lesson_1
    )

    letters_button.grid(
        column=0,
        columnspan=1,
        row=1,
        sticky=(tkinter.W, tkinter.E)
    )
    numbers_button.grid(
        column=0,
        columnspan=1,
        row=2,
        sticky=(tkinter.W, tkinter.E)
    )
    lesson_1_button.grid(
        column=0,
        columnspan=1,
        row=3,
        sticky=(tkinter.W, tkinter.E)
    )

def letters():
    """
    Creates all the letters buttons
    """
    letter_screen = InitializeScreen(FRAMES[2], 3)
    letter_screen.back_to_main_program()
    current_frame = tkinter.Frame(FRAMES[2], width=100, height=100, bg="gray")
    SideFrame(current_frame)

    def all_buttons_forget():
        """
        Configurates all buttons
        """
        letters_1_button.config(relief=RAISED, bg="gray95")
        letters_2_button.config(relief=RAISED, bg="gray95")
        letters_3_button.config(relief=RAISED, bg="gray95")
        letters_4_button.config(relief=RAISED, bg="gray95")

    class Letters:
        """
        Creates intro and quiz buttons uppon creation
        """
        def __init__(self, LN):
            self.lesson_number = LN

            intro_1_button = Button(
                current_frame,
                text=f"Letters {LN}: Introduction Part 1",
                font=("Helvetica", 25),
                command=intro_1(LN)
            )
            intro_1_button.grid(column=1, columnspan=2, row=1)
            intro_1_button.config(bg="gold")

            intro_2_button = Button(
                current_frame,
                text=f"Letters {LN}: Introduction Part 2",
                font=("Helvetica", 25),
                command=intro_2(LN)
            )
            intro_2_button.grid(column=1, columnspan=2, row=2)
            intro_2_button.config(bg="gold")

            quiz_button = Button(
                current_frame,
                text=f"Letters {LN}: Quiz",
                font=("Helvetica", 25),
                command=quiz(LN)
            )
            quiz_button.grid(column=1, columnspan=2, row=3)
            quiz_button.config(bg="gold")

    def intro_1(LN):
        """
        Intro One to Letters
        """
        module = __import__(f"Letters_{LN}_Intro_1")
        func = getattr(module, f"letters_{LN}_intro_1")
        func()

    def intro_2(LN):
        """
        Intro Two to Letters
        """
        module = __import__(f"Letters_{LN}_Intro_2")
        func = getattr(module, f"letters_{LN}_intro_2")
        func()

    def quiz(LN):
        """
        Quiz for Letters
        """
        module = __import__(f"Letters_{LN}_Quiz")
        func = getattr(module, f"letters_{LN}_quiz")
        func()

    def letters_1():
        """
        Sets `letters_1_button` as the selected button
        """
        all_buttons_forget()
        letters_1_button.config(relief=SUNKEN, bg="gold")

    def letters_2():
        """
        Sets `letters_2_button` as the selected button
        """
        all_buttons_forget()
        letters_2_button.config(relief=SUNKEN, bg="gold")

    def letters_3():
        """
        Sets `letters_3_button` as the selected button
        """
        all_buttons_forget()
        letters_3_button.config(relief=SUNKEN, bg="gold")

    def letters_4():
        """
        Sets `letters_4_button` as the selected button
        """
        all_buttons_forget()
        letters_4_button.config(relief=SUNKEN, bg="gold")

    def about_letters():
        """
        Displays information about the arabic alphabet
        """
        messagebox.showinfo("About this Section", "The arabic alphabet consists of 28 letters which have been divided into four sections for ease of learning. Each section introduces two sets of letters and then provides a quiz on those letters. The section on letters is then concluded with a listening comphrension and spelling test.")

    def additional_resources():
        """
        Displays additional information, if needed
        """
        imageroot = f"{ROOT_FILE_NAME}Lessons\\Letters\\AdditionalResources\\"
        image = f"{imageroot}Arabic_Alaphabet.png"
        photo = PhotoImage(file=image)

        canvas = Canvas(tkinter.Toplevel(), height=566, width=850)
        canvas.grid(column=0, row=0)

        alphabet_photo = canvas.create_image(0, 0, image=photo, anchor=NW)
        alphabet_photo.image = photo

    about_button = Button(
        current_frame,
        text="About",
        font=("Helvetica", 25),
        command=about_letters
    )
    about_button.grid(column=1, columnspan=1, row=0, sticky=(tkinter.W, tkinter.E))

    letters_1_button = Button(
        current_frame,
        text="Letters 1",
        font=("Helvetica", 25),
        command=letters_1
    )
    letters_1_button.grid(column=0, columnspan=1, row=1, sticky=(tkinter.W, tkinter.E))

    letters_2_button = Button(
        current_frame,
        text="Letters 2",
        font=("Helvetica", 25),
        command=letters_2
    )
    letters_2_button.grid(column=0, columnspan=1, row=2, sticky=(tkinter.W, tkinter.E))

    letters_3_button = Button(
        current_frame,
        text="Letters 3",
        font=("Helvetica", 25),
        command=letters_3
    )
    letters_3_button.grid(column=0, columnspan=1, row=3, sticky=(tkinter.W, tkinter.E))

    letters_4_button = Button(
        current_frame,
        text="Letters 4",
        font=("Helvetica", 25),
        command=letters_4
    )
    letters_4_button.grid(column=0, columnspan=1, row=4, sticky=(tkinter.W, tkinter.E))

    listening_test_button = Button(
        current_frame,
        text="Listening Test",
        font=("Helvetica", 25),
        command=Letters_Listening_Test.letters_listening_test
    )
    listening_test_button.grid(column=0, columnspan=1, row=5, sticky=(tkinter.W, tkinter.E))

    additional_resources_button = Button(
        current_frame,
        text="Additional Resources",
        font=("Helvetica", 25),
        command=additional_resources
    )
    additional_resources_button.grid(column=1, columnspan=2, row=5, sticky=(tkinter.W, tkinter.E))

def Numbers():
    """
    Creates numbers buttons
    """
    number_screen = InitializeScreen(FRAMES[3], 4)
    number_screen.back_to_main_program()

    current_frame = tkinter.Frame(FRAMES[3], width = 100, height = 100, bg = "gray")
    SideFrame(current_frame)

    def all_buttons_forget():
        """
        Configures all buttons
        """
        numbers_1_button.config(relief=RAISED, bg="gray95")
        license_plate_decoder_button.config(relief=RAISED, bg="gray95")

    def numbers_1():
        """
        Creates numbers_1 button
        """
        all_buttons_forget()
        numbers_1_button.config(relief=SUNKEN, bg="gold")

        numbers_1_intro_1_button = Button(
            current_frame,
            text="Intro to Numbers: Part 1",
            font=("Helvetica", 25),
            command=Numbers_1_Intro_1.numbers_1_intro_1
        )
        numbers_1_intro_1_button.grid(column=1, columnspan=1, row=1)
        numbers_1_intro_1_button.config(bg="gold")

        numbers_1_intro_2_button = Button(
            current_frame,
            text="Intro to Numbers: Part 2",
            font=("Helvetica", 25),
            command=Numbers_1_Intro_2.numbers_1_intro_2
        )
        numbers_1_intro_2_button.grid(column=1, columnspan=1, row=2)
        numbers_1_intro_2_button.config(bg="gold")

    def about_numbers():
        """
        Shows information about the numbers section
        """
        messagebox.showinfo("About this Section", "Though arabic letters are written right to left, arabic numbers are still written left to right. In this numbers section, the individual arabic numbers are introduced in two sets: 0-4, and 5-9.")

    about_button = Button(
        current_frame,
        text="About",
        font=("Helvetica", 25),
        command=about_numbers
    )
    about_button.grid(column=1, columnspan=1, row=0, sticky=(tkinter.W, tkinter.E))

    numbers_1_button = Button(
        current_frame,
        text="Intro to Numbers",
        font=("Helvetica", 25),
        command=numbers_1
    )
    numbers_1_button.grid(column=0, columnspan=1, row=1, sticky=(tkinter.W, tkinter.E))
    numbers_1_button.config(highlightcolor="gold")

    license_plate_decoder_button = Button(
        current_frame,
        text="License Plate Decoder",
        font=("Helvetica", 25),
        command=Numbers_License_Plate_Decoder.license_plate_game
    )
    license_plate_decoder_button.grid(column=0, columnspan=1, row=2, sticky=(tkinter.W, tkinter.E))
    license_plate_decoder_button.config(highlightcolor="gold")

def lesson_1():
    """
    Displays the first lesson
    """
    lesson_screen = InitializeScreen(FRAMES[4], 5)
    lesson_screen.back_to_main_program()

    current_frame = tkinter.Frame(FRAMES[4], width=100, height=100, bg="gray")

    def about_letters():
        """
        Displays information about personal pronouns
        """
        messagebox.showinfo("About this Section", "Lesson 1 focuses on learning personel pronouns.")

    about_button = Button(
        current_frame,
        text="About",
        font=("Helvetica", 25),
        command=about_letters
    )
    about_button.grid(column=1, columnspan=1, row=0, sticky=(tkinter.W, tkinter.E))

    lesson_1_button = Button(
        current_frame,
        text="Lesson 1",
        font=("Helvetica", 25),
        command=Lesson_1_Part_1.lesson_1_part_1
    )
    lesson_1_button.grid(column=0, columnspan=1, row=2, sticky=(tkinter.W, tkinter.E))
    lesson_1_button.config(highlightcolor="gold")

#################################################################3
ROOT_FILE_NAME = "C:\\LearningArabic\\LiblibArriby\\"
IMAGE_PATH = f"{ROOT_FILE_NAME}Icon\\"
MENU_BACKROUND_IMAGES = f"{IMAGE_PATH}menue_screen_images\\"
ICON_TITLES = f"{IMAGE_PATH}menue_screen_images\\"

opening_screen = None

MASTER = Tk()
MASTER.title("LibLib Arriby") # Label the root GUI window.
##master.geometry('700x900+0+0')
##master.attributes("-fullscreen", True) #This would make it full screen

#Define window size
WINDOW_WIDTH = MASTER.winfo_screenwidth() - 100
WINDOW_HEIGHT = MASTER.winfo_screenheight() - 100

NUMBER_OF_FRAMES = 6 #This is the number of frames
FRAMES = []  # This includes frames for all questions
for _ in range(0, NUMBER_OF_FRAMES):
    frame = tkinter.Frame(
        MASTER,
        width=WINDOW_WIDTH,
        height=WINDOW_HEIGHT,
        borderwidth=2,
        relief="sunken"
    )
    frame.grid(column=0, row=0, sticky=(tkinter.W, tkinter.E))
  # frame.grid_remove()
    frame.grid_propagate(False) #Turns off  autoshrinking of Frame widget
    FRAMES.append(frame)

initialize_main_menue()

MASTER.mainloop()
\$\endgroup\$
  • \$\begingroup\$ Linny, thank you so much for taking the time to provide the insight that you did. I know I had a lot of code to work with so I value your effort that much more. I'm going to start implementing the changes now my self and then when I think I'm done I'll compare against what you've done to see if there are things I missed. Hopefully this helps me to actually learn how to code better. \$\endgroup\$ – Jack Duane Aug 5 at 11:00
  • \$\begingroup\$ Linny, I have one question on your above code. Almost at the end of your updated code you have a line: opening_screen = None What is the purpose of this line? \$\endgroup\$ – Jack Duane Aug 5 at 12:46
2
\$\begingroup\$

Let's talk about numbers.

import Numbers_1_Intro_1, Numbers_1_Intro_2
import Lesson_1_Part_1
import Personal_Pronouns_Intro_1, Personal_Pronouns_Intro_2
import Greetings_Intro_1, Greetings_Intro_2, Greetings_Intro_3
import Greetings_Conversation_1, Greetings_Conversation_2

Remember when I said this?

You can't put 50 questions/answers in the code itself, it becomes a mess. A royal mess. Don't even try.

You made a good start by extracting the data from the main code. That was a very good first step. Now we need to make a couple more steps before this turns into a royal mess anyway. Because what happens when you've reached 50 lessons? What are your imports going to look like?

So, the golden rule about numbers is we shouldn't be using numbers. Huh? Yes. Part of this is not to use magic numbers. If you ever see get_adjusted_data(original_data * 42 - 2) in someone's code, that's bad. But there're more ways for numbers to be bad. Like in your imports.

import Lesson_1_Part_1, Lesson_1_Part_2, Lesson_2_Part_1, Lesson_2_Part_2, Lesson_3_Part_1, Lesson_3_Part_2, Lesson_4_Part_1, Lesson_4_Part_2, Lesson_5_Part_1, Lesson_5_Part_2

What's in those files that you need one for each lesson? Can't you reuse the code? And what would that look like?

Hard to say without taking a look at those files, but let's use another part of your code where the same thing goes wrong as an example. For example, your lessons.

Instead of creating lesson_1, lesson_2, etc., create lessons[] and append the lessons to the list (like they did here with answers.append(answer)), using the index of the list to access different lessons. Your letter, lesson and number variable names all contain numbers. That's going to cost you in maintainability eventually.

def letters_1():
    all_buttons_forget()
    letters = Letters(1)
    letters_1_button.config(relief = SUNKEN, bg = "gold")

def letters_2():
    all_buttons_forget()
    letters = Letters(2)
    letters_2_button.config(relief = SUNKEN, bg = "gold")

def letters_3():
    all_buttons_forget()
    letters = Letters(3)
    letters_3_button.config(relief = SUNKEN, bg = "gold")

def letters_4():
    all_buttons_forget()
    letters = Letters(4)
    letters_4_button.config(relief = SUNKEN, bg = "gold")

And your buttons do the same thing:

letters_2_button = Button(
    current_frame, text = "Letters 2",
    font=("Helvetica", 25), command = letters_2)
letters_2_button.grid(column = 0, columnspan = 1, row = 2,
                      sticky = (tkinter.W, tkinter.E))

I don't have the time to fully understand what global magic is going on here, but could we possibly do something like this instead?

def all_buttons_forget(colour):
    for button in letters_buttons[num]:
        button.config(relief = RAISED, bg = colour)

def create_button(num):
    # Figure out max amount of buttons per row and column based on number
    # Beware of the magic numbers
    # Set these in a constant somewhere, or as member variable of the class it goes under
    # Assuming 6 buttons per col_span, 5 per row
    calculated_col = (num // 6)
    calculated_col_span = calculated_col + 1
    calculated_row = num % 5

    # Font should be in a constant as well
    Button(
            current_frame, text = "Letters {0}".format(num),
            font=("Helvetica", 25), command = commands[num])
        letters_buttons[num].grid(column = calculated_col, columnspan = calculated_col_span, row = calculated_row,
                              sticky = (tkinter.W, tkinter.E))

def create_letter(num, rel, colour):
    all_buttons_forget("gray95")
    letters[num] = Letters(num)
    letters[num].button.config(relief = rel, bg = colour)

create_button(4)
create_letter(4, SUNKEN, "gold")

Functions and variables should never have indexes in their name. Sometimes a number is hard to avoid, but indexes aren't. Using indexes in a variable indicates you're using the wrong datatype.

Instead, put them in a list, a dictionary, an enum, anything. Such containers are indexable, iterable, searchable and a lot more useful than a lot of individual functions. Also a lot easier to maintain. Say you got 50 `lett

Take an interest in modulo operators % and how integer division works // in Python 3. Pass arguments around. Set a couple as constant in the class or globally so you can remember what it means.

Those colours for example. You could make a dictionary:

COLOURS = {
    'active' : "gold",
    'inactive': "gray95"
}

create_letter(4, SUNKEN, COLOURS['active'])

Perhaps you want to set SUNKEN as part of the 'active' configuration as well? Use a nested dictionary:

CONFIGS = {
    'active':
        {'rel': SUNKEN, 'colour': "gold"},
    'inactive':
        {'rel': RAISED, 'colour': "gray95"}
}

create_letter(4, CONFIGS['active']['rel'], CONFIGS['active']['colour'])

Can we improve this even further? Absolutely. We could simplify the configuration, or put your entire configuration in a JSON. Plenty of options. But that's for another review.

It's all a bit rough around the edges and your code hangs so together it's impractical to rewrite it all (especially since you really should be combining this advice with Linny's, so it's going to look different anyway). But I hope to convey the idea here. It still isn't as good as I'd like it, but hey, we're taking steps here. Never try to run the entire race in one step.

And remember. If you can make this piece of code more generic, you can do the same to the rest of your code. Including your imports.

\$\endgroup\$
  • 1
    \$\begingroup\$ This is fantastic, I've already started rewriting the letters_1, letters_2, etc. but this goes a long way - even with re-writing the other modules so that I don't have to import an individual one for each lesson set. What you said about functions not having numbers makes sense, but I'd never seen that anywhere before. Good to know. I hope to work on re-writing this section of code but hopefully in a short while I'll have an updated version available. You and Linny have given me a lot to work with here! \$\endgroup\$ – Jack Duane Aug 8 at 10:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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