2
\$\begingroup\$

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()
 
toolic
14.4k5 gold badges29 silver badges201 bronze badges
asked Apr 30, 2016 at 0:21
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

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------------------------#
answered Mar 4 at 11:22
\$\endgroup\$

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.