manager = MyFileManager()
filename_button = tkinter.Button(window, text="Browse", command=manager.get_filename)
save_filename_button = tkinter.Button(window, text="Browse", command=manager.get_save_file)
go_button = tkinter.Button(window, text="GO!", command=lambda: manager.save_users_list(
list_name.get()))
- Notice how in my examples above I
use_underscores
instead ofcamelCase
for variable names. This is the Python convention. - When thinking of variable names, err on the side of being too verbose rather than too brief:
list_name_label
is a much better name thanlistNameLBL
. - You mostly do a good job of this, but for GUI widgets remember to append the type of widget in the variable name. You forgot to do this on lyouryour
filename
button widget.
filename_button = tkinter.Button(window, text="Browse", command=manager.get_filename)
save_filename_button = tkinter.Button(window, text="Browse", command=manager.get_save_file)
go_button = tkinter.Button(window, text="GO!", command=lambda: manager.save_users_list(
list_name.get()))
- Notice how in my examples above I
use_underscores
instead ofcamelCase
for variable names. This is the Python convention. - When thinking of variable names, err on the side of being too verbose rather than too brief:
list_name_label
is a much better name thanlistNameLBL
. - You mostly do a good job of this, but for GUI widgets remember to append the type of widget in the variable name. You forgot to do this on lyour
filename
button widget.
manager = MyFileManager()
filename_button = tkinter.Button(window, text="Browse", command=manager.get_filename)
save_filename_button = tkinter.Button(window, text="Browse", command=manager.get_save_file)
go_button = tkinter.Button(window, text="GO!", command=lambda: manager.save_users_list(
list_name.get()))
- Notice how in my examples above I
use_underscores
instead ofcamelCase
for variable names. This is the Python convention. - When thinking of variable names, err on the side of being too verbose rather than too brief:
list_name_label
is a much better name thanlistNameLBL
. - You mostly do a good job of this, but for GUI widgets remember to append the type of widget in the variable name. You forgot to do this on your
filename
button widget.
Structure
Firstly, try not to use the global
keyword. This keyword should really be used only if you HAVE to. A simple solution for this is to implement a class that wraps around all of the functions:
class MyFileManager(object):
def __init__(self, filename='', save_filename=''):
self.filename = filename
self.save_filename = save_filename
# Re-named `program` function
def save_users_list(self, list_name):
...
def get_filename(self):
self.filename = askopenfilename(filetypes=(("Text files", "*.txt"),
("All files", "*.*")))
def get_save_file(self):
self.save_filename = asksaveasfilename(filetypes=(("Text files", "*.txt"),
("All files", "*.*")))
Now your button declarations would look like this (see here for the reasoning behind using lambda
for go_button
):
filename_button = tkinter.Button(window, text="Browse", command=manager.get_filename)
save_filename_button = tkinter.Button(window, text="Browse", command=manager.get_save_file)
go_button = tkinter.Button(window, text="GO!", command=lambda: manager.save_users_list(
list_name.get()))
Naming
- Notice how in my examples above I
use_underscores
instead ofcamelCase
for variable names. This is the Python convention. - When thinking of variable names, err on the side of being too verbose rather than too brief:
list_name_label
is a much better name thanlistNameLBL
. - You mostly do a good job of this, but for GUI widgets remember to append the type of widget in the variable name. You forgot to do this on lyour
filename
button widget.
program
Function
Your orignal function has a few problems/tweaks that can be made:
- The name is very ambiguous. Update it to better reflect what the function does.
- You used the
with
statement to openfilename
. However, when you opensaveName
you don't. Because of that you forgot toclose()
that file. ALWAYS usewith
when accessing files as it will save you these mistakes. - I prefer to always say which mode we are opening files in. So add the mode (
'r'
) to thewith
statement. - Try to make the amount of time a file is open the smallest it can be. So instead of having
filename
open for the duration of the function only keep it open when creatinglongList
. - Instead of doing multiple string concatenations, use
format()
to create yourfinList
. - There is no need for the parens around variables.
Here is my version of your program
function. Assume it is inside the class I outlined above:
def save_users_list(self, list_name):
with open(self.filename, 'r') as file:
numbers_list = [line.strip() for line in file.readLines()]
with open(self.save_filename, 'w') as file:
file.write('{} = {}'.format(list_name, str(numbers_list)))
As a final general comment, take a look at PEP8, the official Python style guide. It will help your code look and feel more Pythonic.