3
\$\begingroup\$

This is a program that clears the desktop and also undoes the clear. How does the code look? Could I make it look better?

import wx 
import os
import shutil
import time

class Window(wx.Frame):
    def __init__(self, parent, id): 
        self.Desktop = "C:/Users/Name/Desktop/" and "C:/Users/Public/Desktop/"
        self.Dfiles = "C:/ Desktop_Files/"                
        no_caption = wx.DEFAULT_FRAME_STYLE | wx.STAY_ON_TOP 
        wx.Frame.__init__(self, parent, id, title="no_caption", size=(300,97), style=no_caption)            
        self.panel=wx.Panel(self)
        self.Butt_Clear = wx.Button(self.panel, -1, label="Clear Desktop", pos=(0,0), size=(100,70))
        self.Butt_Clear.Bind(wx.EVT_BUTTON, self.ClearDesk, self.Butt_Clear)
        self.Butt_Undo = wx.Button(self.panel, -1, label="Undo Clear", pos=(185,0), size=(100,70))
        self.Butt_Undo.Bind(wx.EVT_BUTTON, self.UndoClear, self.Butt_Undo)                      
    def ClearDesk(self, e):        
        MainDesktop = os.listdir(self.Desktop)       
        MainDesktop.remove('desktop.ini')

        for Desktop_Path_Names in MainDesktop:  
            Desktop_Path = os.path.join(self.Desktop, Desktop_Path_Names)           
            shutil.move(Desktop_Path, self.Dfiles)

    def UndoClear(self, e): 
        Dfolder = os.listdir(self.Dfiles) 
        Dfolder.remove('desktop.ini')
        for Desktop_Folder_Path_Names in Dfolder:
            Dfolder_Path = os.path.join(self.Dfiles, Desktop_Folder_Path_Names)
            shutil.move(Dfolder_Path, self.Desktop)

if __name__=='__main__':
    app=wx.App(False)
    frame=Window(parent=None, id=-1)
    frame.Show()
    app.MainLoop()
\$\endgroup\$
6
\$\begingroup\$
  • Your class name is misleading; it says "Window" as if you are trying to draw a window for a GUI purpose, but that is not what that class actually performs. So you should rename it to something else meaningful to enhance the readability of your program.

  • Parameters and arguments constitute a level of abstraction which is different from that of the function (or class construction) they are injected it. You should minimize them as much as you can, or avoid them totally if that is possible. In your case, the initializer of your class has a parent parameter which is never used (in your main, you assigned None to it), so it is better you get rid of it as it is useless for your case.

  • Always in the initializer of your class, you used the id parameter only here:

    wx.Frame.__init__(self, parent, id, title="no_caption", size=(300,97), style=no_caption)
    

    So for the same reason as mentioned through the second bullet above, I suggest you to get rid of it. Thus, your initializer should become like this:

    def __init__(self):
        ...
        ...
    
  • The work is not finished with your __init__(). You should clean it by keeping only what is necessary there, otherwise delegate all the remaining tasks for functions to take care of them. I say your __init__() is not clean because you mix widget creation and settings with some configurations and constants that your application needs. So I suggested you to move this:

    self.Desktop = "C:/Users/Name/Desktop/" and "C:/Users/Public/Desktop/"
    self.Dfiles = "C:/ Desktop_Files/"                
    

    into a different function. The only "constant" you need to keep in your class initializer is the parent parameter, but you do not use it. In case you used it, it would be the only element that deserves to sit and have its own place in the initializer by running this instruction self.parent = parent

  • For the same reason, I suggest you to create a separate function to save the settings of your application:

    def configure_gui(self):
        no_caption = wx.DEFAULT_FRAME_STYLE | wx.STAY_ON_TOP
    
  • Please do not create widgets within your class initializer. Delegate that task, again, for a separate function:

    def create_widgets(self):
        wx.Frame.__init__(self, parent, id, title="no_caption", size=(300,97), style=no_caption)            
        self.panel=wx.Panel(self)
        self.Butt_Clear = wx.Button(self.panel, -1, label="Clear Desktop", pos=(0,0), size=(100,70))
        self.Butt_Clear.Bind(wx.EVT_BUTTON, self.ClearDesk, self.Butt_Clear)
        self.Butt_Undo = wx.Button(self.panel, -1, label="Undo Clear", pos=(185,0), size=(100,70))
        self.Butt_Undo.Bind(wx.EVT_BUTTON, self.UndoClear, self.Butt_Undo)      
    
  • In Python, we don't like camelCase and CamelCase naming conventions that you probably used in Java, JavaScript or elsewhere. Please follow the naming conventions as described in PEP8.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.