Making a list from user input

Question

I have wrote a program that lets the user open up their text file that would look something like this:

1
2
3
4
5
6

It would then let the user enter a name for the list such as "numbers" The program would then let the user choose where they want to save the list. The user would then click go and the program would turn their input into a list like this:

numbers = ['1', '2', '3', '4', '5', '6']

This is the code that I wrote:

import tkinter
from tkinter import *
from tkinter.filedialog import askopenfilename
from tkinter.filedialog import asksaveasfilename
from tkinter.messagebox import showerror
from tkinter import filedialog
import sys

def program():
    global saveName
    with open(fname) as input_file:
        longList = [line.strip() for line in input_file.readlines()]
        finListName = listName.get()
        finList = (finListName) + (" = ") + str(longList)
        txtFile = open((saveName),"w")
        txtFile.write(finList)

#Window
window = tkinter.Tk()
window.title("Python List Maker")
window.geometry("300x170")

#Label
fileSelectLBL = tkinter.Label(window, text="Please select your file:")
fileSelectLBL.pack()

#Button
def load_file():
    global fname
    fname = askopenfilename(filetypes=(("Text files", "*.txt"),
                                       ("All files", "*.*") ))
filename = tkinter.Button(window, text="Browse", command = load_file)
filename.pack()

#List Name
listNameLBL = tkinter.Label(window,text="Please enter what you want to call the list:")
listNameLBL.pack()
listName = Entry(window)
listName.pack()

#Save List
saveListLBL = tkinter.Label(window,text="Please select where you want to save the list:")
saveListLBL.pack()
def save_file():
    global saveName
    saveName = asksaveasfilename(filetypes=(("Text files", "*.txt"),
                                       ("All files", "*.*") ))
Savename = tkinter.Button(window, text="Browse", command = save_file)
Savename.pack()

#Go
goBTN = tkinter.Button(window, text="GO!", command = program)
goBTN.pack()

#Main Loop
window.mainloop()

Now although this code works, it is really messy. I am not that new to Python but my code is much longer and complex than what it could/should be. This was my first ever go at a program with a GUI, all of my other programs have just been run through the shell.

I am just looking for some general feedback on how I can improve this, and make it shorter and more efficient.

Answer 1

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):

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()))

Naming

• Notice how in my examples above I use_underscores instead of camelCase 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 than listNameLBL.
• 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.

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 open filename. However, when you open saveName you don't. Because of that you forgot to close() that file. ALWAYS use with 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 the with 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 creating longList.
• Instead of doing multiple string concatenations, use format() to create your finList.
• 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.