3
\$\begingroup\$

I have been learning python for 1.5 month now I am learning in my free time between job and other life commitments so my progress may be slow. It may take time for me to reply but I am interested in opinions and reviews on my code in order to get better faster and identify any mistakes I have made.

I would appreciate ideas to improve the code or any thoughts you may have. I would also appreciate reviews focusing on:

  • code readability
  • code efficiency

Learning language program with few different subjects and languages included.

I have taken all my data from the internet, with translations through Google Translate so the data may be inaccurate. I am also not an artist, please excuse the graphics I've created.

Buttons.py

from tkinter import Button

class Button(Button):
    def __init__(self,master, command=None):
        super().__init__(master=master)
        self['borderwidth'] = 0
        self['bg'] = self.master['bg']
        self['command'] = command
        self['activebackground'] = self.master['bg']

class LanguageSettingsButton(Button):
    def __init__(self,master, image, command):
        super().__init__(master=master,command=command)
        self['image'] = image

class MenuButton(Button):
    def __init__(self,master, text, command,font):
        super().__init__(master=master,command=command)
        self['text'] = text
        self['font'] = (font,25)
        self['fg'] = '#040814'

Data.py

from PIL import ImageTk,Image
import pandas as pd
import numpy as np
import json

class Data():
    def __del__(self):
        json.data = {
            'native_lang' : self.native_lang,
            'active_language' : self.active_language
        }
        self.prase_data['native_lang'] = self.native_lang
        self.prase_data[f'active_language_{self.native_lang}'] = self.active_language
        self.settings_file.write(json.dumps(self.prase_data,indent=4))

    def __init__(self):
        self.supported_languages = ['en','fr','de','es','ja','it','he']
        self.subjects = ['Top 100','Top 400','Top 1000','Animals','Food']

        self.font = 'David'
        self.font_2 = 'FuturistStencil'
        self.colors = {
            'main' : '#b469ff',
            'white' : '#FAF9F6'
        }

        self.word_list = None
        self.progress_list = None

        self.logos = {}
        self.dictionary = {}
        self.borders = {}
        self.generals = {}

        self.settings_file = open('resources/pylang/settings.json',mode='r')
        self.prase_data = json.load(self.settings_file)
        self.native_lang = self.prase_data['native_lang']
        self.active_language = self.prase_data[f'active_language_{self.native_lang}']
        self.settings_file = open('resources/pylang/settings.json', mode='w')

        self.load_images()
    def load_images(self):
        for lang in self.supported_languages:
            self.logos[lang] = ImageTk.PhotoImage(file=f'resources/pylang/images/{lang}.png')
        self.logos['main'] = ImageTk.PhotoImage(Image.open(fp='resources/pylang/images/main_logo.png'))
        self.logos['main_small'] = ImageTk.PhotoImage(Image.open(fp='resources/pylang/images/main_logo.png').resize(size=(140,35)))


        self.borders['border'] = ImageTk.PhotoImage(file='resources/pylang/images/border.png')
        self.borders['big_up'] = ImageTk.PhotoImage(file='resources/pylang/images/big_border_upside.png')
        self.borders['big_down'] = ImageTk.PhotoImage(file='resources/pylang/images/big_border_downside.png')
        self.borders['selected'] = ImageTk.PhotoImage(file='resources/pylang/images/selected_border.png')
        self.borders['non'] =ImageTk.PhotoImage(file='resources/pylang/images/non_selected_border.png')
        self.borders['shade'] = ImageTk.PhotoImage(file='resources/pylang/images/border_shade.png')
        self.borders['loadbar'] = ImageTk.PhotoImage(file='resources/pylang/images/loadbar_border.png')

        self.generals['return'] = ImageTk.PhotoImage(file='resources/pylang/images/return.png')
        self.generals['start'] = ImageTk.PhotoImage(Image.open(fp='resources/pylang/images/start.png').resize(size=(60,60)))
        self.generals['start_s'] = ImageTk.PhotoImage(Image.open(fp='resources/pylang/images/start.png').resize(size=(49,40)))
        self.generals['loadbar'] = Image.open(fp='resources/pylang/images/loadbar.png')
        self.generals['check_button'] = ImageTk.PhotoImage(file='resources/pylang/images/check_button.png')
        self.generals['cancel_button'] = ImageTk.PhotoImage(file='resources/pylang/images/cancel_button.png')

    def load_menu(self,lang):
        temp_csv = pd.read_csv('resources/pylang/menu.csv')
        return temp_csv[lang]

    def load_language(self,lang):
        temp_csv = pd.read_csv('resources/pylang/languages.csv')
        return temp_csv[lang]

    def get_loadbar(self,percentage):
        temp_loadbar = self.generals['loadbar'].crop((0,0,155*(percentage/100),15))
        temp_loadbar = ImageTk.PhotoImage(temp_loadbar)
        return temp_loadbar

    def get_lists(self,subject,lang):
        self.word_list = pd.read_csv(f'resources/pylang/words/{subject}.csv').copy()
        self.progress_list = pd.read_csv(f'resources/pylang/progress/{subject}_{self.native_lang}_progress.csv').copy()

        for n,word in enumerate(self.progress_list[lang]):
            self.word_list[self.word_list[lang] == word] = np.nan

    def update_progress(self,subject,learned_lang,word):
        word_list = self.progress_list[learned_lang].dropna()

        self.progress_list.loc[len(word_list),learned_lang] = word
        self.progress_list.to_csv(path_or_buf=f'resources/pylang/progress/{subject}_{self.native_lang}_progress.csv',index=False)

        self.word_list[self.word_list[learned_lang] == word] = np.nan

SubFrame.py

from tkinter import Frame,Canvas,Label
from Buttons import *

class SubFrame(Frame):
    def __init__(self,master,data,headline):
        super().__init__(master=master,width=master.width,height=master.height,bg=master['bg'])
        self.width = master.width
        self.height = master.height
        self.pack_propagate(False)
        self.pack()

        self.data = data

        self.header_frame = Frame(master=self,width=self.width,height=self.height*0.08,
                                  bg=self.data.colors['white'])
        self.header_frame.pack_propagate(False)
        self.header_frame.pack()

        self.return_button = LanguageSettingsButton(master=self.header_frame, image=self.data.generals['return'],
                                                    command=self.destroy)
        self.return_button.pack(side='left', pady=5, padx=10)

        self.logo = Canvas(master=self.header_frame, width=140, height=35, highlightthickness=0,
                           bg=self.header_frame['bg'])
        self.logo.create_image(70, 35 / 2, image=self.data.logos['main_small'])
        self.logo.pack(side='right', pady=5, padx=10)

        self.header = Label(master=self.header_frame, text=headline, font=(self.data.font, 20),
                            bg=self.header_frame['bg'])
        self.header.pack(pady=5)

        self.main_frame = Frame(master=self, width=self.width, height=self.height - 50, bg=self['bg'])
        self.main_frame.pack()

    def clear_frames(self):
        self.main_frame.destroy()
        self.header_frame.destroy()

mainwindow.py

import pandas as pd
from tkinter import Tk,Frame,Canvas
from Data import Data
from Buttons import LanguageSettingsButton,MenuButton
from NewLearnFrame import NewLearnFrame
from ContinueLearnFrame import ContinueLearnFrame

MAIN_COLOR = '#b469ff'
MAIN_COLOR_LIGHT = '#cd9cff'
SECOND_COLOR = '#040814'
SECOND_COLOR_LIGHT = '#0c193e'
OFF_WHITE = '#FAF9F6'

class MainWindow(Tk):
    def __init__(self, title, width, height):
        #window settings
        super().__init__()
        self.title(title)
        self.geometry('1x1+600+200')
        self.minsize(width=width, height=height)
        self.width = width
        self.height = height
        self['bg'] = MAIN_COLOR

        #general settings
        self.data = Data()

        #settings_frame
        self.settings_container = Frame(master=self,bg=self['bg'],borderwidth=0)
        self.settings_container.place(x=self.width*0.94, y=self.height*0.01)

        self.language_button = LanguageSettingsButton(master=self.settings_container,
                                                      image=self.data.logos[self.data.native_lang],
                                                      command=self.open_lang_menu)
        self.language_button.grid(column=0,row=0)


        #menu_frame
        self.menu = self.data.load_menu(self.data.native_lang)
        item_spaces = 25

        self.menu_container = Frame(master=self,bg=OFF_WHITE)
        self.menu_container.place(x=self.width/2, y=self.height/2,anchor='center')

        self.logo = Canvas(self.menu_container,width=240,height=60,highlightthickness=0,bg=self.menu_container['bg'])
        self.logo.create_image(0,0,image = self.data.logos['main'],anchor='nw')
        self.logo.pack(pady=item_spaces/2,anchor='center')

        self.learn_new_button = MenuButton(self.menu_container,text=self.menu[0],font=self.data.font,
                                           command=lambda : NewLearnFrame(self,self.data))
        self.learn_new_button.pack(pady=item_spaces)

        self.continue_learn_button = MenuButton(self.menu_container, text=self.menu[1],font=self.data.font,
                                                command=lambda: ContinueLearnFrame(master=self,data=self.data))
        self.continue_learn_button.pack(pady=item_spaces)

        self.exit_button = MenuButton(self.menu_container,font=self.data.font,text=self.menu[2],command=self.destroy)
        self.exit_button.pack(pady=item_spaces)


    def open_lang_menu(self):
        if len(self.settings_container.children) <= 1:
            row_parm = 0
            for language in self.data.supported_languages:
                if language != self.data.native_lang:
                    row_parm += 1
                    LanguageSettingsButton(master=self.settings_container, image=self.data.logos[language],
                                           command=lambda lang=language: self.update_language(lang)).grid(column=0,row=row_parm)
        else:
            self.close_lang_menu()

    def update_language(self,lang):
        self.data.native_lang = lang
        self.language_button.config(image=self.data.logos[lang])
        self.menu = self.data.load_menu(self.data.native_lang)
        self.learn_new_button['text'] = self.menu[0]
        self.continue_learn_button['text'] = self.menu[1]
        self.exit_button['text'] = self.menu[2]
        self.close_lang_menu()
        self.data.active_language = self.data.prase_data[f'active_language_{self.data.native_lang}']

        for subject in self.data.subjects:
            try:
                open(f'resources/pylang/progress/{subject}_{self.data.native_lang}_progress.csv')
            except:
                df = pd.DataFrame(columns=self.data.supported_languages)
                df.to_csv(f'resources/pylang/progress/{subject}_{self.data.native_lang}_progress.csv')

    def close_lang_menu(self):
        for child in self.settings_container.winfo_children()[1:]:
            child.destroy()

if __name__ == '__main__':

    MainWindow = MainWindow('Pylang',800,600)

    MainWindow.mainloop()

NewLearnFrame.py

from tkinter import Canvas
from Buttons import LanguageSettingsButton
from SubFrame import SubFrame

class NewLearnFrame(SubFrame):
    def __init__(self,master,data):
        super().__init__(master=master, data=data, headline='Choose Language')

        self.lang_cards = []
        self.selected_card = None
        self.language_list = self.data.load_language(self.data.native_lang)

        self.main_frame.grid_propagate(False)
        for x in range(0,int(len(self.data.supported_languages)/3)+1):
            if x < 3:
                self.main_frame.columnconfigure(index=x,pad=25)
            self.main_frame.rowconfigure(index=x,pad=25)

        row = 0
        col = 0
        for index,lang in enumerate(self.data.supported_languages):
            card = Canvas(master=self.main_frame, width=240, height=160,bg=self.main_frame['bg'],highlightthickness=0)
            card.create_image(120, 80, image=self.data.borders['non'],tag='border')
            card.create_text(120,30,text=self.language_list[index],font=(self.data.font,25))
            card.create_image(120, 80, image=self.data.logos[lang])
            if lang == self.data.native_lang or lang in self.data.active_language:
                card.create_image(120,80,image=self.data.borders['shade'],tag='shade')

            card.grid(column=col,row=row)
            card.bind("<Button-1>",lambda e,i=index:self.select_card(i))
            col += 1
            if col == 3:
                col = 0
                row += 1
            self.lang_cards.append(card)

        self.start_learn_button = LanguageSettingsButton(master=self.main_frame,image=self.data.generals['start'],
                                                         command=lambda : self.add_language(self.selected_card))
        self.start_learn_button.grid(column=2,row=row,sticky='se',pady=10)

    def add_language(self,language):
        self.data.active_language.append(self.data.supported_languages[self.lang_cards.index(language)])
        self.destroy()

    def select_card(self,index):
        if self.data.native_lang != self.data.supported_languages[index] and self.data.supported_languages[index] not in self.data.active_language:
            if self.selected_card == None:

                self.selected_card = self.lang_cards[index]
                self.selected_card.itemconfig('border',image=self.data.borders['selected'])
                return

            self.selected_card.itemconfig('border',image=self.data.borders['non'])
            self.selected_card = self.lang_cards[index]
            self.selected_card.itemconfig('border', image=self.data.borders['selected'])

ContinueLearnFrame.py

import random
import numpy as np
from SubFrame import SubFrame
from Buttons import LanguageSettingsButton
from tkinter import Canvas

class ContinueLearnFrame(SubFrame):
    def __init__(self,master,data,selected_lang=None):
        super().__init__(master=master, data=data, headline='Choose Language')

        self.main_frame.grid_propagate(False)
        self.main_frame.pack_propagate(False)

        self.cards = []
        self.cards_loadbar = []
        self.buttons = []
        self.words = {}
        self.selected_lang = selected_lang

        if self.selected_lang == None:
            self.load_languages()

    def learn_language(self,lang):
        self.selected_lang = lang
        self.load_subjects()

    def erase(self):
        for card in self.cards:
            card.destroy()
        for button in self.buttons:
            button.destroy()

        self.cards = []
        self.buttons = []
        self.cards_loadbar = []

    def load_languages(self):
        self.erase()

        self.header.config(text="Choose Language")
        self.return_button.config(command=self.destroy)

        for col in range(0, 3):
            self.main_frame.columnconfigure(index=col, pad=25)
        for row in range(0, int(len(self.data.active_language) / 3) + 1):
            self.main_frame.rowconfigure(index=row, pad=25)

        col = 0
        row = 0
        for index, lang in enumerate(self.data.active_language):
            card = Canvas(master=self.main_frame, width=240, height=160, bg=self.main_frame['bg'], highlightthickness=0)
            card.create_image(120, 80, image=self.data.borders['border'], tag='border')
            card.create_text(120, 50, text=self.data.active_language[index], font=(self.data.font, 25))
            card.create_image(120, 110, image=self.data.logos[lang])
            card.grid(column=col, row=row)
            card.bind("<Button-1>", lambda e, l=lang: self.learn_language(l))
            if col < 2:
                col += 1
            else:
                row += 1
                col = 0
            self.cards.append(card)

    def load_subjects(self):

        self.erase()

        self.header.config(text="Choose Subject")
        self.return_button.config(command=self.load_languages)


        for col in range(0, 3):
            self.main_frame.columnconfigure(index=col, pad=25)
        for row in range(0, int(len(self.data.subjects) / 3) + 1):
            self.main_frame.rowconfigure(index=row, pad=25)
        col = 0
        row = 0
        for index, subj in enumerate(self.data.subjects):
            self.data.get_lists(subject=subj,lang=self.selected_lang)

            total = len(self.data.word_list[self.selected_lang])
            progress = len(self.data.progress_list[self.selected_lang].dropna())
            precent = round((progress/total)*100)

            self.cards_loadbar.append(self.data.get_loadbar(precent))
            card = Canvas(master=self.main_frame, width=240, height=160, bg=self.main_frame['bg'], highlightthickness=0)
            if len(self.data.word_list[self.selected_lang].dropna()) != 0:
                card.create_image(120, 80, image=self.data.borders['loadbar'], tag='border')
                card.create_image(14, 135, image=self.cards_loadbar[-1], tag='loadbar',anchor='w')
                card.create_text(205, 135, text=f'{precent}%', font=(self.data.font, 16))
                card.create_text(120, 90, text=f'{progress}/'
                                               f'{total}',
                                 font=(self.data.font, 25))
                card.bind("<Button-1>", lambda e, sub=self.data.subjects[index]: self.load_learn_program(sub))
            else:
                card.create_image(120, 80, image=self.data.borders['border'], tag='border')
                card.create_text(120, 135, text=f'COMPLETED', font=(self.data.font, 16))
                card.create_text(120, 90, text='100%',
                                 font=(self.data.font, 25))

            card.create_text(120, 40, text=self.data.subjects[index], font=(self.data.font, 30))

            card.grid(column=col, row=row)

            if col < 2:
                col += 1
            else:
                row += 1
                col = 0
            self.cards.append(card)

    def load_learn_program(self,subject):
        self.data.get_lists(subject=subject,lang=self.selected_lang)

        def load_cards(self):
            card_upside = Canvas(master=self.main_frame, width=600, height=400, bg=self.main_frame['bg'],
                                 highlightthickness=0)
            card_upside.create_image(300, 200, image=self.data.borders['big_up'], tag='border')
            card_upside.create_text(300, 100,
                                    text=f'{self.data.load_language(lang=self.data.native_lang)[self.data.supported_languages.index(self.selected_lang)]}',
                                    font=(self.data.font, 50),
                                    tags='headline')
            card_upside.create_text(300,250,text='',font=(self.data.font, 30),tags='word')
            card_upside.bind("<Button-1>", lambda e: swap_cards(self, 0))
            card_upside.pack(pady=40)
            self.cards.append(card_upside)


            card_downside = Canvas(master=self.main_frame, width=600, height=400, bg=self.main_frame['bg'],
                                   highlightthickness=0)
            card_downside.create_image(300, 200, image=self.data.borders['big_down'], tag='border')
            card_downside.create_text(300, 100,
                                    text=f'{self.data.load_language(lang=self.data.native_lang)[self.data.supported_languages.index(self.data.native_lang)]}',
                                    font=(self.data.font, 50),
                                    tags='headline')
            card_downside.create_text(300,250,text='',font=(self.data.font, 30),tags='word')
            card_downside.bind("<Button-1>", lambda e: swap_cards(self, 1))
            self.cards.append(card_downside)

        def swap_cards(self, index):
            if index == 0:
                self.cards[0].pack_forget()
                self.cards[1].pack(pady=40)
            else:
                self.cards[0].pack(pady=40)
                self.cards[1].pack_forget()

        def update_cards(self):
            while(True):
                try:
                    random_word = random.choice(self.data.word_list[self.selected_lang].dropna().tolist())
                    if random_word != np.nan and random_word != np.NaN:
                        current_word_index = self.data.word_list.index[self.data.word_list[self.selected_lang] == random_word].tolist()[0]
                        self.words['native'] = self.data.word_list[self.data.native_lang][current_word_index]
                        self.words['learned'] = self.data.word_list[self.selected_lang][current_word_index]
                        self.cards[0].itemconfigure('word', text=f'{self.words['learned']}')
                        self.cards[1].itemconfigure('word', text=f'{self.words['native']}')
                        break
                except IndexError as e:
                    self.cards[0].itemconfigure(tagOrId='headline',text='Congratulations')
                    self.cards[0].itemconfigure(tagOrId='word',text='Subject completed')
                    for button in self.buttons:
                        button.destroy()
                    break

            try:
                self.cards[0].pack_info()
            except:
                swap_cards(self,1)

        def approval_word(self):
            self.data.update_progress(subject=subject,learned_lang=self.selected_lang,word=self.words['learned'])
            update_cards(self)

        self.erase()

        self.header.config(text=f"{subject}")
        self.return_button.config(command=self.load_subjects)


        load_cards(self)
        update_cards(self)


        check_button = LanguageSettingsButton(master=self.main_frame, image=self.data.generals['check_button'],
                                               command=lambda : approval_word(self))
        check_button.place(x=300, y=500, anchor='center')
        self.buttons.append(check_button)

        cancel_button = LanguageSettingsButton(master=self.main_frame, image=self.data.generals['cancel_button'],
                                               command=lambda : update_cards(self))
        cancel_button.place(x=500,y=500,anchor='center')
        self.buttons.append(cancel_button)

- full code - Github

\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

The good stuff

  • The interface and the data model are not intricated: the data model is not dependent on Tkinter at all
  • Use of if __name__ == "__main__": clause
  • The size of the methods and classes is OK in most cases
  • The naming is globally OK

Style

Make sure to take a look at PEP 8 to properly format your code. There are inconsistencies in:

  • The spacing between the classes/functions (two newlines expected) and methods (one newline expected)
  • The spacing after the commas (one space expected)
  • The spacing in comments (two spaces before #, one after)
  • ...

If you want to properly format your code without having to worry, you could use a tool like black to do that for you.

Your files should be named with the snake_case convention: buttons.py, data.py, new_learn_frame.py...

About type annotations

As a beginner in python, this should not be your first priority, but you need to know that type hints exist, and that they can help you.

Main advantages:

  • Autocompletion: when using a proper code editor or IDE, type annotations help a lot for an accurate autocompletion.
  • Documentation: when reading code of other people, or code that you wrote a while ago, they help to understand what classes/functions/methods expect as inputs, and what they return.
  • Code correctness: type hints help detect errors statically, in your IDE or code editor, or in external tools like mypy.

Inner functions

Using inner functions (load_cards, update_cards...) that take a self argument is quite surprising in ContinueLearnFrame.py. These functions could be replaced by simple methods:

class ContinueLearnFrame(SubFrame):
    ...

    def _swap_cards(self, index: int) -> None:
        ...

    def _load_cards(self) -> None:
        ...
        card_upside.bind("<Button-1>", lambda e: self._swap_cards(0))
        ...
        card_downside.bind("<Button-1>", lambda e: self._swap_cards(1))
    
    def _update_cards(self) -> None:
        ...
        except:
            self._swap_cards(1)

    def _approval_word(self, subject: str) -> None:
        ...
        self._update_cards()

    def load_learn_program(self, subject: str) -> None:
        ...
        self._load_cards()
        self._update_cards()
        check_button = LanguageSettingsButton(
            master=self.main_frame,
            image=self.data.generals["check_button"],
            command=lambda: self._approval_word(subject),
        )
        ...
        cancel_button = LanguageSettingsButton(
            master=self.main_frame,
            image=self.data.generals["cancel_button"],
            command=self._update_cards,
        )

The _ in front of the method names is just a standard way (enforced by PEP 8) to indicate that the method is private, and should not be accessed outside the class.

Equality test with None

if self.selected_lang == None:
    ...

None is a singleton, and this test should be replaced with:

if self.selected_lang is None:
    ...

See this answer for more information.

Overwriting names

There are some spots where variables are unintentionally overwritten, and that could lead to bugs.

MainWindow

MainWindow = MainWindow('Pylang',800,600)

Here you are overwriting the MainWindow class, and it is not possible to create another independent MainWindow anymore. Also, the variable that stores the instantiated MainWindow should follow the snake_case naming convention:

main_window = MainWindow("Pylang", 800, 600)
main_window.mainloop()

Button

from tkinter import Button

class Button(Button):
    ...

Here, the tkinter.Button class of tkinter is also overwritten, which makes it impossible to use it in the following parts of the module. You should consider doing:

import tkinter as tk


class Button(tk.Button):
    ...

Unused code

Some variables/methods are never used:

  • Data.font_2
  • Data.dictionary
  • SubFrame.clear_frames()

They clutter the code and can slow down the execution (probably not here though). They can be detected with specialized tools like vulture, or more generic linters like pylint or flake8.

About lambdas

Variable that are not used intentionally should be prefixed with _ and there is no need to over-complicate things here (ContinueLearnFrame.py):

card.bind("<Button-1>", lambda e, l=lang: self.learn_language(l))

could be written:

card.bind("<Button-1>", lambda _e: self.learn_language(lang))

Use well-defined data structures instead of dicts

In the Data class, some attributes would deserve a proper data structure, for example logos, borders, generals... Using simple dictionaries in this case is error prone, as it is easy to mistype a key when accessing the elements.

By declaring a dataclass, you get access to autocompletion, and other tools can help you detect errors in your code without having to execute anything (mypy).

The generals attribute would look like:

...
from dataclasses import dataclass
...

@dataclass
class GenericImages:
    return_img: ImageTk.PhotoImage
    start: ImageTk.PhotoImage
    start_s: ImageTk.PhotoImage
    loadbar: Image.Image
    check_button: ImageTk.PhotoImage
    cancel_button: ImageTk.PhotoImage


class Data:
    def __init__(self) -> None:
        ...
        self.generals: GenericImages | None = None
        ...

    def load_images(self) -> None:
        ...
        self.generals = GenericImages(
            return_img=ImageTk.PhotoImage(file="resources/pylang/images/return.png"),
            start=ImageTk.PhotoImage(
                Image.open(fp="resources/pylang/images/start.png").resize(size=(60, 60))
            ),
            start_s=ImageTk.PhotoImage(
                Image.open(fp="resources/pylang/images/start.png").resize(size=(49, 40))
            ),
            loadbar=Image.open(fp="resources/pylang/images/loadbar.png"),
            check_button=ImageTk.PhotoImage(
                file="resources/pylang/images/check_button.png"
            ),
            cancel_button=ImageTk.PhotoImage(
                file="resources/pylang/images/cancel_button.png"
            ),
        )

Accessing the attributes is done like one would do with a standard class:

self.generals.check_button

Catching broad exception

In the MainWindow there is a confusing piece of code:

try:
    open(f'resources/pylang/progress/{subject}_{self.data.native_lang}_progress.csv')
except:
    df = pd.DataFrame(columns=self.data.supported_languages)
    df.to_csv(f'resources/pylang/progress/{subject}_{self.data.native_lang}_progress.csv')

Here:

  • The except clause is too broad: this can hide bugs, and it makes your intentions unclear
  • Using open should lead to a call to close at some point (or a with statement should be used)
  • The generation of the path string is duplicated

What you probably want to do is:

progress_csv_path = 'resources/pylang/progress/{subject}_{self.data.native_lang}_progress.csv'
if not os.path.exists(progress_csv_path):
    df = pd.DataFrame(columns=self.data.supported_languages)
    df.to_csv(progress_csv_path)

Conclusion

There is a lot to say (and I'm sure I would have more to say if I dug deeper), but for a progress of a month and a half, it's pretty good. Keep up the good work!

\$\endgroup\$
3
  • \$\begingroup\$ Wow, first of all, thanks for the review. I really appreciate it. I have taken all the notes. I will publish my next project soon, and I will include all these notes. I would like it if you could have some time and review my next project as well, so I will have more factors to improve on. \$\endgroup\$
    – sean
    Commented Jan 9 at 22:22
  • \$\begingroup\$ I posted my new project. If you have any free time, I would like you to review my code, as I got good points from you last time. I have used all of your tips except data structures, as I can see why knowing this idea might help in big projects or collaborative projects to prevent typing errors and get achieve completion. But is it really necessary to define a data structure in small projects? If so, let me know, so I can implement data structures in my next projects. \$\endgroup\$
    – sean
    Commented Jan 11 at 23:04
  • \$\begingroup\$ I would say that the point on the data structure is the most important of my post. It is a really good habit to define the data your application depends on. Those data structures can define internal concepts: the state of your application, objects that are closely related to each other that must be transferred between modules (like your images), etc... Keeping a definition at a single location helps a lot when it comes to centralizing the core concepts of a project, writing documentation, etc... Defining them early is important, as your IDE cannot help you when everything is dynamic. \$\endgroup\$
    – rdesparbes
    Commented Jan 12 at 18:03

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.