5
\$\begingroup\$

Below code produces a basic text editor that I have written using tkinter with python. I tried to be as well structured as I can but in some parts I have been lazy about it.

Classes that are used:

  • MainWindow: I tried to manage anything topmost level using this class but I was unable to tell if something was 'topmost level' or not at some parts such as changing the title using a children's method like new_file.
  • MainFrame: This class was really the non-configuration parts of the 'topmost level' stuff, and was created with concerns of putting it in another Toplevel widget. It holds the information on widget's configurations and their methods. But again, at the end I don't really see a strict line between this and MainWindow.
  • MainMenu: This class is the structure class to hold sub-menus.
  • FileMenu: This is a sub-menu class to be assigned as file related parts of the MainMenu.
  • EditMenu: This is a sub-menu class to be assigned as edit related parts of the MainMenu.
  • AutoScrollbar: This exists for smart scrollbar visibility.

Here's the code:

#required for anything GUI related
import tkinter as tk
#required for fileopen and save options
from tkinter import filedialog
#required for file's basename
import os
#required for save_before_leave basically
from tkinter import messagebox
#Nae Unicode text entering is missing
class MainWindow(tk.Tk):
 def __init__(self):
 super().__init__()
 self.title("Untitled - NaePad")
 self.mainFrame = MainFrame(self)
 self.mainFrame.pack(fill="both", expand=True)
 self.mainFrame.text.focus_set()
 self.protocol("WM_DELETE_WINDOW", lambda : self.save_before_leave(self.destroy))
 def save_before_leave(self, callback, *args):
 textBuffer = self.mainFrame.text.get('1.0', 'end-1c')
 #if there's a current file
 if self.mainFrame.curFilePath:
 #if it content doesn't match the current buffer
 if self.mainFrame.curFileCont != textBuffer:
 response = messagebox.askyesnocancel("NaePad - Unsaved File", "Do you want to save before leaving?")
 #if response is yes
 if response:
 self.mainFrame.save_file()
 callback()
 #if response is no
 elif response is False:
 callback()
 else:
 callback()
 else:
 #if there's any text in the buffer
 if textBuffer:
 response = messagebox.askyesnocancel("NaePad - Unsaved File", "Do you want to save before leaving?")
 #If it's a yes
 if response:
 self.mainFrame.save_as_file()
 callback()
 #if it's a no
 elif response is False:
 callback()
 else:
 callback()
 #required in order to prevent "tagbinds" from happening
 return "break"
class MainFrame(tk.Frame):
 def __init__(self, master):
 super().__init__(master)
 self.master = master
 #creates and displays the Menu
 self.menu = MainMenu(master)
 master.config(menu=self.menu)
 #path of the current file
 self.curFilePath = ''
 #current file
 self.curFileCont = ''
 self.text = tk.Text(self, wrap="none")
 #color scheme
 self.text.config(bg='#282c34',fg='#abb2bf', selectbackground='#3e4451')
 self.text.grid(row=0, column=0, sticky="nsew")
 self.rowconfigure(0, weight=1)
 self.columnconfigure(0, weight=1)
 self.scroll_bar_config()
 self.menu_command_config()
 self.key_binds_config()
 #scrollbar configurations for self.text widget
 def scroll_bar_config(self):
 self.scrollY = AutoScrollbar(self, orient="vertical", command=self.text.yview)
 self.scrollY.grid(row=0, column=1, sticky="nsew")
 self.text['yscrollcommand'] = self.scrollY.set
 self.scrollX = AutoScrollbar(self, orient="horizontal", command=self.text.xview)
 self.scrollX.grid(row=1, column=0, sticky="nsew")
 self.text['xscrollcommand'] = self.scrollX.set
 def menu_command_config(self):
 #New
 self.menu.file.entryconfig(0, command=lambda : self.master.save_before_leave(self.new_file))
 #Open...
 self.menu.file.entryconfig(1, command=lambda : self.master.save_before_leave(self.open_file))
 #Save
 self.menu.file.entryconfig(2, command=self.save_file)
 #Save as
 self.menu.file.entryconfig(3, command=self.save_as_file)
 #Cut
 self.menu.edit.entryconfig(0, command=self.cut)
 #Copy
 self.menu.edit.entryconfig(1, command=self.copy)
 #Paste
 self.menu.edit.entryconfig(2, command=self.paste)
 #Delete
 self.menu.edit.entryconfig(3, command=self.delete)
 def key_binds_config(self):
 self.text.bind('<Control-n>', lambda event : self.master.save_before_leave(self.new_file))
 self.text.bind('<Control-N>', lambda event : self.master.save_before_leave(self.new_file))
 self.text.bind('<Control-o>', lambda event : self.master.save_before_leave(self.open_file))
 self.text.bind('<Control-O>', lambda event : self.master.save_before_leave(self.open_file))
 self.text.bind('<Control-s>', self.save_file)
 self.text.bind('<Control-S>', self.save_file)
 self.text.bind('<Control-Shift-s>', self.save_as_file)
 self.text.bind('<Control-Shift-S>', self.save_as_file)
 def new_file(self, *args):
 self.text.delete("1.0", "end")
 self.curFilePath = ''
 self.curFileCont = ''
 #update window name to file name BAD NAE BAD
 self.master.title("Untitled - NaePad")
 def open_file(self, *args):
 #get filepath from user with gui
 filePath = filedialog.askopenfilename(filetypes=(("Text files", "*.txt"),("All files", "*.*")))
 # if filePath is selected
 if filePath:
 try:
 #open only UTF-8 encoded files
 with open(filePath, encoding="UTF-8") as f:
 self.text.delete("1.0", "end")
 self.curFileCont = f.read()
 self.text.insert("1.0", self.curFileCont)
 #if it's not UTF-8 then
 except UnicodeDecodeError:
 #open as ANSI encoding
 with open(filePath, encoding="ANSI") as f:
 self.text.delete("1.0", "end")
 self.curFileCont = f.read()
 self.text.insert("1.0", self.curFileCont)
 #update window name to file name BAD NAE BAD
 self.master.title(os.path.basename(f.name) + " - NaePad")
 #update current file path
 self.curFilePath = filePath
 def save_file(self, *args):
 #if there's already a file
 if self.curFilePath:
 try:
 #open only UTF-8 encoded files
 with open(self.curFilePath, 'w', encoding="UTF-8") as f:
 self.curFileCont = self.text.get('1.0', 'end-1c')
 f.write(self.curFileCont)
 #if it's not UTF-8 then
 except UnicodeDecodeError:
 #open as ANSI encoding
 with open(self.curFilePath, 'w', encoding="ANSI") as f:
 self.curFileCont = self.text.get('1.0', 'end-1c')
 f.write(self.curFileCont)
 #update window name to file name BAD NAE BAD
 self.master.title(os.path.basename(f.name) + " - NaePad")
 else:
 self.save_as_file()
 def save_as_file(self, *args):
 self.curFilePath = filedialog.asksaveasfilename(defaultextension=".txt", filetypes=(("Text files", "*.txt"),("All files", "*.*")))
 #Checks wether a file is selected or not
 if self.curFilePath:
 self.save_file()
 def cut(self):
 #Is anything selected?
 if self.text.tag_ranges(tk.SEL):
 self.text.clipboard_clear()
 #append to cleared clipboard the (selection)
 self.text.clipboard_append(self.text.get(tk.SEL_FIRST, tk. SEL_LAST))
 self.text.delete(tk.SEL_FIRST, tk. SEL_LAST)
 def copy(self):
 #Is anything selected?
 if self.text.tag_ranges(tk.SEL):
 self.text.clipboard_clear()
 #append to cleared clipboard the (selection)
 self.text.clipboard_append(self.text.get(tk.SEL_FIRST, tk. SEL_LAST))
 def paste(self):
 #Is anything selected?
 if self.text.tag_ranges(tk.SEL):
 #keeping a reference on where the selection starts
 selFirstIndex = self.text.index(tk.SEL_FIRST)
 #removing selection first
 self.text.delete(tk.SEL_FIRST, tk. SEL_LAST)
 #copying from clipboard
 self.text.insert(selFirstIndex, self.text.clipboard_get())
 else:
 self.text.insert(tk.INSERT, self.text.clipboard_get())
 def delete(self):
 #Is anything selected?
 if self.text.tag_ranges(tk.SEL):
 self.text.delete(tk.SEL_FIRST, tk. SEL_LAST)
#MainMenu object that contains Sub-menus
class MainMenu(tk.Menu):
 def __init__(self, master):
 super().__init__(master, tearoff=0)
 self.master = master
 #create Menu Options
 self.file = FileMenu(self)
 self.add_cascade(label="File", menu=self.file)
 self.edit = EditMenu(self)
 self.add_cascade(label="Edit", menu=self.edit)
#Menu class that handles File Operations
class FileMenu(tk.Menu):
 def __init__(self, master):
 super().__init__(master, tearoff=0)
 self.master = master
 self.add_command(label="New Ctrl + N")
 self.add_command(label="Open... Ctrl + O")
 self.add_command(label="Save Ctrl + S")
 self.add_command(label="Save As... Ctrl + Shift + S")
 #add the line before exit
 self.add_separator()
 #destroy's the grandparent, which is assumed to be a toplevel BAD NAE BAD
 self.add_command(label="Exit Alt + F4", command=lambda : master.master.save_before_leave(self.master.master.destroy))
#Menu class that handles editorial operations
class EditMenu(tk.Menu):
 def __init__(self, master):
 super().__init__(master, tearoff=0)
 self.add_command(label="Cut Ctrl + X")
 self.add_command(label="Copy Ctrl + C")
 self.add_command(label="Paste Ctrl + V")
 self.add_command(label="Delete Delete")
#http://effbot.org/zone/tkinter-autoscrollbar.htm
class AutoScrollbar(tk.Scrollbar):
 # a scrollbar that hides itself if it's not needed. only
 # works if you use the grid geometry manager.
 def set(self, lo, hi):
 if float(lo) <= 0.0 and float(hi) >= 1.0:
 self.grid_remove()
 else:
 self.grid()
 tk.Scrollbar.set(self, lo, hi)
def test():
 with open(__file__, "rU") as f:
 root.mainFrame.text.insert("1.0", f.read())
if __name__ == "__main__":
 root = MainWindow()
 #test()
 root.mainloop()

Review Concern(s):

  • My main concern is to code in a, easy to read, efficient, and well structured manner while still learning the language and concepts such as OOP. Feel free to mention tiniest issue or improvement that comes to your mind, as I am a beginner and I probably need it.

My own critique:

There are some inconsistencies between arguments to similar methods such as insert("1.0",...) and insert('1.0', ...) I want to structure my "" and '' usage so that I can better write and understand code but I am undecided which one to use in some situations.

I question the very necessity of MainFrame class, I think I should have removed it and assigned its children directly to MainWindow class instead.

Using self.master.something feels pretty wrong let alone using master.master.something but I am not entirely sure on what to do when there's a method that needs access to both a parent and a grandchildren's parameters, I would assume that's a parent's method than a children's.

I am most proud of the save_file and save_as_file methods as I believe they co-depend on each other very well. Still there's the issue of their level though.

There's the issue of encodings, which turned out to be a much harder problem than I initially anticipated. I had to use try, except for an easy way out.

Mast
13.8k12 gold badges56 silver badges127 bronze badges
asked Dec 4, 2017 at 2:39
\$\endgroup\$
3
  • 1
    \$\begingroup\$ You missed a line in the Rollback. Did you know there's an actual 'rollback' button in every revision of your question except the last? \$\endgroup\$ Commented Dec 4, 2017 at 15:05
  • \$\begingroup\$ @Mast Thanks for the help but I've read it's available if you have full edit privilege. Is it not? Though according to this the right thing to do is to flag as other and ask for a rollback. \$\endgroup\$ Commented Dec 4, 2017 at 15:42
  • 1
    \$\begingroup\$ I thought there was an exception for questions you own, but you could well be correct. Anyway, we got it nicely sorted out now. Feel free to find us in chat if you ever need help :-) \$\endgroup\$ Commented Dec 4, 2017 at 16:46

2 Answers 2

2
\$\begingroup\$

Few things I would bring your attention to:

  • naming. It is recommended by the PEP8 styleguide and generally agreed to follow the lower_case_with_underscores variable and function naming style. You have violated it on multiple occasions - e.g. mainFrame, curFilePath, curFileCont etc.
  • comments are over-used. Code without comments might feel like an extreme and somewhat opinion-based, but it is generally a great way of thinking about self-documented code. Just to highlight a few spots - I don't see the usefulness of #path of the current file before the self.curFilePath = '' line - especially if you would name a variable current_file_path. And, you can and should convert some of the comments into proper Documentation Strings
  • code repetition. Don't repeat yourself. Think of extracting a method out of the save_before_leave() method - look how the bodies of the two if blocks are identical to each other
answered Dec 4, 2017 at 12:37
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Based on your last point I noticed that they can be identical(initially they called different save methods) and then written a new method response_handle for that part, and updated my code. Will work on other points as well. Thanks a lot for the review. \$\endgroup\$ Commented Dec 4, 2017 at 14:44
  • 3
    \$\begingroup\$ @Nae Please see this Meta post, you might want to rollback your last edit codereview.meta.stackexchange.com/questions/1763/… \$\endgroup\$ Commented Dec 4, 2017 at 14:52
-1
\$\begingroup\$
#required for anything GUI related
import tkinter as tk
#required for fileopen and save options
from tkinter import filedialog
#required for file's basename
import os
#required for save_before_leave basically
from tkinter import messagebox
#Nae Unicode text entering is missing
class MainWindow(tk.Tk):
 def __init__(self):
 super().__init__()
 self.title("Untitled - NaePad")
 self.mainFrame = MainFrame(self)
 self.mainFrame.pack(fill="both", expand=True)
 self.mainFrame.text.focus_set()
 self.protocol("WM_DELETE_WINDOW", lambda : self.save_before_leave(self.destroy))
 def save_before_leave(self, callback, *args):
 textBuffer = self.mainFrame.text.get('1.0', 'end-1c')
 #if there's a current file
 if self.mainFrame.curFilePath:
 #if it content doesn't match the current buffer
 if self.mainFrame.curFileCont != textBuffer:
 self.response_handle(callback)
 else:
 callback()
 else:
 #if there's any text in the buffer
 if textBuffer:
 self.response_handle(callback)
 else:
 callback()
 #required in order to prevent "tagbinds" from happening
 return "break"
 def response_handle(self, callback):
 response = messagebox.askyesnocancel("NaePad - Unsaved File", "Do you want to save before leaving?")
 #if response is yes
 if response:
 self.mainFrame.save_file()
 callback()
 #if response is no
 elif response is False:
 callback()
class MainFrame(tk.Frame):
 def __init__(self, master):
 super().__init__(master)
 self.master = master
 #creates and displays the Menu
 self.menu = MainMenu(master)
 master.config(menu=self.menu)
 #path of the current file
 self.curFilePath = ''
 #current file
 self.curFileCont = ''
 self.text = tk.Text(self, wrap="none")
 #color scheme
 self.text.config(bg='#282c34',fg='#abb2bf', selectbackground='#3e4451')
 self.text.grid(row=0, column=0, sticky="nsew")
 self.rowconfigure(0, weight=1)
 self.columnconfigure(0, weight=1)
 self.scroll_bar_config()
 self.menu_command_config()
 self.key_binds_config()
 #scrollbar configurations for self.text widget
 def scroll_bar_config(self):
 self.scrollY = AutoScrollbar(self, orient="vertical", command=self.text.yview)
 self.scrollY.grid(row=0, column=1, sticky="nsew")
 self.text['yscrollcommand'] = self.scrollY.set
 self.scrollX = AutoScrollbar(self, orient="horizontal", command=self.text.xview)
 self.scrollX.grid(row=1, column=0, sticky="nsew")
 self.text['xscrollcommand'] = self.scrollX.set
 def menu_command_config(self):
 #New
 self.menu.file.entryconfig(0, command=lambda : self.master.save_before_leave(self.new_file))
 #Open...
 self.menu.file.entryconfig(1, command=lambda : self.master.save_before_leave(self.open_file))
 #Save
 self.menu.file.entryconfig(2, command=self.save_file)
 #Save as
 self.menu.file.entryconfig(3, command=self.save_as_file)
 #Cut
 self.menu.edit.entryconfig(0, command=self.cut)
 #Copy
 self.menu.edit.entryconfig(1, command=self.copy)
 #Paste
 self.menu.edit.entryconfig(2, command=self.paste)
 #Delete
 self.menu.edit.entryconfig(3, command=self.delete)
 def key_binds_config(self):
 self.text.bind('<Control-n>', lambda event : self.master.save_before_leave(self.new_file))
 self.text.bind('<Control-N>', lambda event : self.master.save_before_leave(self.new_file))
 self.text.bind('<Control-o>', lambda event : self.master.save_before_leave(self.open_file))
 self.text.bind('<Control-O>', lambda event : self.master.save_before_leave(self.open_file))
 self.text.bind('<Control-s>', self.save_file)
 self.text.bind('<Control-S>', self.save_file)
 self.text.bind('<Control-Shift-s>', self.save_as_file)
 self.text.bind('<Control-Shift-S>', self.save_as_file)
 #method handling file operations with encoding in mind
 def file_operation(self, file_path, file_opt='r'):
 #if file_path exists
 if file_path:
 try:
 #open only UTF-8 encoded files
 with open(file_path, file_opt, encoding="UTF-8") as f:
 self.data_operation(f, file_opt)
 #if it's not UTF-8 then
 except UnicodeDecodeError:
 #open as ANSI encoding
 with open(file_path, fil_opt, encoding="ANSI") as f:
 self.data_operation(f, file_opt)
 #update current file path
 self.curFilePath = file_path
 #update window name to file name BAD NAE BAD
 self.master.title(os.path.basename(f.name) + " - NaePad")
 #If it's a save operation and there's no file_path
 elif ((not file_path) and (file_opt == 'w')):
 self.save_as_file()
 #data writing and reading method
 def data_operation(self, file_data, file_opt):
 #read data
 if file_opt == 'r':
 self.text.delete("1.0", "end")
 self.curFileCont = file_data.read()
 self.text.insert("1.0", self.curFileCont)
 #write data
 elif file_opt == 'w':
 self.curFileCont = self.text.get('1.0', 'end-1c')
 file_data.write(self.curFileCont)
 def new_file(self, *args):
 self.text.delete("1.0", "end")
 self.curFilePath = ''
 self.curFileCont = ''
 #update window name to file name BAD NAE BAD
 self.master.title("Untitled - NaePad")
 def open_file(self, *args):
 #get filepath from user with gui
 filePath = filedialog.askopenfilename(filetypes=(("Text files", "*.txt"),("All files", "*.*")))
 self.file_operation(filePath, 'r')
 def save_file(self, *args):
 self.file_operation(self.curFilePath, 'w')
 def save_as_file(self, *args):
 self.curFilePath = filedialog.asksaveasfilename(defaultextension=".txt", filetypes=(("Text files", "*.txt"),("All files", "*.*")))
 #Checks wether a file is selected or not
 if self.curFilePath:
 self.save_file()
 def cut(self):
 #Is anything selected?
 if self.text.tag_ranges(tk.SEL):
 self.text.clipboard_clear()
 #append to cleared clipboard the (selection)
 self.text.clipboard_append(self.text.get(tk.SEL_FIRST, tk. SEL_LAST))
 self.text.delete(tk.SEL_FIRST, tk. SEL_LAST)
 def copy(self):
 #Is anything selected?
 if self.text.tag_ranges(tk.SEL):
 self.text.clipboard_clear()
 #append to cleared clipboard the (selection)
 self.text.clipboard_append(self.text.get(tk.SEL_FIRST, tk. SEL_LAST))
 def paste(self):
 #Is anything selected?
 if self.text.tag_ranges(tk.SEL):
 #keeping a reference on where the selection starts
 selFirstIndex = self.text.index(tk.SEL_FIRST)
 #removing selection first
 self.text.delete(tk.SEL_FIRST, tk. SEL_LAST)
 #copying from clipboard
 self.text.insert(selFirstIndex, self.text.clipboard_get())
 else:
 self.text.insert(tk.INSERT, self.text.clipboard_get())
 def delete(self):
 #Is anything selected?
 if self.text.tag_ranges(tk.SEL):
 self.text.delete(tk.SEL_FIRST, tk. SEL_LAST)
#MainMenu object that contains Sub-menus
class MainMenu(tk.Menu):
 def __init__(self, master):
 super().__init__(master, tearoff=0)
 self.master = master
 #create Menu Options
 self.file = FileMenu(self)
 self.add_cascade(label="File", menu=self.file)
 self.edit = EditMenu(self)
 self.add_cascade(label="Edit", menu=self.edit)
#Menu class that handles File Operations
class FileMenu(tk.Menu):
 def __init__(self, master):
 super().__init__(master, tearoff=0)
 self.master = master
 self.add_command(label="New Ctrl + N")
 self.add_command(label="Open... Ctrl + O")
 self.add_command(label="Save Ctrl + S")
 self.add_command(label="Save As... Ctrl + Shift + S")
 #add the line before exit
 self.add_separator()
 #destroy's the grandparent, which is assumed to be a toplevel BAD NAE BAD
 self.add_command(label="Exit Alt + F4", command=lambda : master.master.save_before_leave(self.master.master.destroy))
#Menu class that handles editorial operations
class EditMenu(tk.Menu):
 def __init__(self, master):
 super().__init__(master, tearoff=0)
 self.add_command(label="Cut Ctrl + X")
 self.add_command(label="Copy Ctrl + C")
 self.add_command(label="Paste Ctrl + V")
 self.add_command(label="Delete Delete")
#http://effbot.org/zone/tkinter-autoscrollbar.htm
class AutoScrollbar(tk.Scrollbar):
 # a scrollbar that hides itself if it's not needed. only
 # works if you use the grid geometry manager.
 def set(self, lo, hi):
 if float(lo) <= 0.0 and float(hi) >= 1.0:
 self.grid_remove()
 else:
 self.grid()
 tk.Scrollbar.set(self, lo, hi)
def test():
 with open(__file__, "rU") as f:
 root.mainFrame.text.insert("1.0", f.read())
if __name__ == "__main__":
 root = MainWindow()
 #test()
 root.mainloop()

Update 1: I have written a new method response_handle for the response handle to be used in 2 places in order to reduce repetition as suggested in this answer. Also for a similar reason I've written file_operation and data_operation methods to be used in save_file and open_file methods as they were very similar.

answered Dec 4, 2017 at 15:38
\$\endgroup\$
3
  • 1
    \$\begingroup\$ This is CodeReview you post alternative code, but don't post a review of the code. codereview.meta.stackexchange.com/questions/8403/… \$\endgroup\$ Commented Dec 4, 2017 at 17:17
  • \$\begingroup\$ @Ludisposed I don't understand how this is unfitting in accordance to your link and your other link? \$\endgroup\$ Commented Dec 4, 2017 at 17:32
  • 1
    \$\begingroup\$ You can answer your own code, but don't just dump it here. Explain your reasoning behind the changes should be enough. You can link to points in @alexce answer \$\endgroup\$ Commented Dec 4, 2017 at 17:42

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.