I'm doing a Python course for beginners, and the last assignment was to create a program using shutil and wxpython that can copy new files (created in the last 24 hours) from one folder into another, while leaving the old files alone.
I feel like it's pretty messy, and there are multiple functions that do almost the same thing and could probably be combined. For instance, there are two different functions for printing a directory path into two different text boxes.
Also, I don't think I quite have a handle on when to put variables inside of functions or outside of them, adding to the messiness.
I'd love a critique and some pointers on how the existing code could be simplified and cleaned up.
from wx import *
from datetime import *
import os.path
import shutil
# Today - 24 hours = "yesterday"
yesterday = datetime.now() - timedelta(days=1)
class windowClass(wx.Frame):
# Frame makin'
def __init__(self, *args, **kwargs):
super(windowClass, self).__init__(size=(470,175),*args, **kwargs)
self.basicGUI()
self.Center()
#------------------------WIDGETS & VARIABLES------------------------#
def basicGUI(self):
# Settin' it up
panel = wx.Panel(self)
self.SetTitle('File Transfer Manager')
self.Show(True)
# Menu bar
menuBar = wx.MenuBar()
self.SetMenuBar(menuBar)
# File menu
fileButton = wx.Menu()
menuBar.Append(fileButton, 'File')
# File - "Check/Transfer"
checkItem = wx.MenuItem(fileButton, wx.ID_ANY,"Check/Transfer Files...")
fileButton.AppendItem(checkItem)
self.Bind(wx.EVT_MENU, self.Message, checkItem)
# File - "Quit"
exitItem = wx.MenuItem(fileButton, wx.ID_EXIT,"Quit")
fileButton.AppendItem(exitItem)
self.Bind(wx.EVT_MENU, self.Quit, exitItem)
# Directory textbox labels
srcText = wx.StaticText(panel, -1, "Browse for source directory...",(10,10))
destText = wx.StaticText(panel, -1, "Browse for destination directory...",(10,57))
# Directory textboxes
self.control1 = wx.TextCtrl(panel,size=(200, -1),pos=(10,27), style=wx.TE_READONLY)
self.control2 = wx.TextCtrl(panel,size=(200, -1),pos=(10,74), style=wx.TE_READONLY)
# Browse buttons
srcBtn = wx.Button(panel, label="Browse",pos=(217,26))
srcBtn.Bind(wx.EVT_BUTTON, self.onDir1)
destBtn = wx.Button(panel, label="Browse",pos=(217,73))
destBtn.Bind(wx.EVT_BUTTON, self.onDir2)
# Check/Transfer button
checkBtn = wx.Button(panel, label="Check/Transfer",size=(110,73),pos=(325,26))
checkBtn.Bind(wx.EVT_BUTTON, self.Message)
#------------------------FUNCTIONS------------------------#
# Figure out how many new files are in source directory
def counting(self):
count = 0
for file in os.listdir(self.srcPath):
mod = self.modTime(self.srcPath+"//"+file)
if yesterday < mod:
count = count+1
return count
# Get 'Date Modified' for a file
def modTime(self, filePath):
t = os.path.getmtime(filePath)
return datetime.fromtimestamp(t)
# Dialog box giving number of new files
# and asking if you'd like to transfer them
def Message(self, e):
try:
checkBox = wx.MessageDialog(None, 'There are currently '+str(self.counting())+
' new files in the source folder. Copy them to the destination folder?',
caption='Caption',style=YES_NO|CENTRE, pos=DefaultPosition)
checkAnswer = checkBox.ShowModal()
if checkAnswer == wx.ID_YES:
self.Transfer()
checkBox.Destroy()
except:
print "You failed to specify one or more directories."
# Makes GUI close
def Quit(self, e):
self.Close()
# Where the magic happens
def Transfer(self):
for file in os.listdir(self.srcPath):
mod = self.modTime(self.srcPath+"//"+file)
# Transfer the files that were modified in the last 24 hours
if yesterday < mod:
shutil.copy((self.srcPath+"//"+file), self.destPath)
print self.srcPath+"//"+file+" successfully copied."
# Ignore older files
elif yesterday > mod:
print "File skipped."
print "Operation completed."
# Adds directory paths to the textboxes
def TextBox1(self, path):
self.control1.ChangeValue(path)
def TextBox2(self, path):
self.control2.ChangeValue(path)
# Dialog: Browse for source folder
def onDir1(self, event):
dlg = wx.DirDialog(self, "Choose a directory:",
style=wx.DD_DEFAULT_STYLE
)
if dlg.ShowModal() == wx.ID_OK:
self.srcPath = dlg.GetPath()
self.TextBox1(self.srcPath)
dlg.Destroy()
# Dialog: Browse for destination folder
def onDir2(self, event):
dlg = wx.DirDialog(self, "Choose a directory:",
style=wx.DD_DEFAULT_STYLE
)
if dlg.ShowModal() == wx.ID_OK:
self.destPath = dlg.GetPath()
self.TextBox2(self.destPath)
dlg.Destroy()
# Do it up
def main():
app = wx.App()
windowClass(None)
app.MainLoop()
main()
1 Answer 1
Documentation
The PEP 8 style guide recommends adding docstrings for classes and functions. You can convert comments to docstrings. For example, change:
# Figure out how many new files are in source directory
def counting(self):
to:
def counting(self):
""" Figure out how many new files are in source directory """
Naming
PEP-8 recommends snake_case for function and variable names. For example, change:
def modTime(self, filePath):
to:
def mod_time(self, file_path):
Simpler
The following:
count = count+1
can be simplified using the special assignment operator:
count += 1
You can get rid of the temporary variable t
:
t = os.path.getmtime(filePath)
return datetime.fromtimestamp(t)
using:
return datetime.fromtimestamp(os.path.getmtime(filePath))
Since the e
variable is not used by these functions, it can be deleted:
def Message(self, e):
def Quit(self, e):
Regarding this if/else
:
if yesterday < mod:
elif yesterday > mod:
if it is possible for the case where yesterday == mod
,
then that case should be handled as well.
DRY
In the Transfer
function, this expression is repeated 3 times:
self.srcPath+"//"+file
You can set it to a variable.
Portability
I realize this question was posted many years ago when Python version 2.x was prevalent, but now that it is deprecated, consider porting to 3.x.
I'm not sure if the main guard was supported then, but it is common practice now:
if __name__ == '__main__'
main()
Comments
These comments can be removed since they are not needed:
#------------------------WIDGETS & VARIABLES------------------------#
#------------------------FUNCTIONS------------------------#
Explore related questions
See similar questions with these tags.