Skip to main content
add link to PEP-8
Source Link
  • Definitely have a look at the python logging page and the logging cookbook for some simple patterns on building a logger.

  • Other thoughts are: class Log() doesn't do what it says it is. It alerts, and passes a "download completed" message to the logger. I'd recommend separating the functionality there and renaming the class to what it actually does.

  • Move the code from Start() into the __main__ - you're not making it a class so there's no point on having that code separate.

  • For this (spelling error btw):

     if not self.location:
         self.loaction = os.getcwd()
    

you might want to just call self.GetSaveDir() as that's doing the prompting to get the value of the location to save, correct?

  • D.R.Y. here (also saving a log message to say "refer to the log is kind of redundant, right?):

     except Exception as err:
         AddLog(err)
         Log.error("Exception, please refer to log")
    
  • With:

     urlYT = self.getUrlE.get()
     if urlYT == "":
         Log.error("Please enter a url")
         return
    

Perhaps you could make this a loop (or check for 'exit' to abort) on the response. Something like (untested):

    while not (urlYT = self.getUrlE.get())
        Alerter.error("Please enter a URL or use 'exit' to abort.")
    if "exit" == urlYT:
        return
  • The function def CreateUI(self, master): is huge. You should be aiming for as few lines as possible in a function. I'd suggest refactoring each operation inside that function into their own separate functions so def CreateUI(self, master): becomes smaller. For example:

     def CreateUI(self, master):
         build_urls()
         build_file_format_getter()
         set_form_formats()
         set_location_buttons()
         set_logger()
         set_progress_bar()
         set_download_button()
    

The reason is, if you have a bug, you're not searching 50 lines for the cause. You only have to look at a specific function where the problem is.

  • Finally, the naming of your variables aren't PEP8in line with PEP8. Hope this can help :-)
  • Definitely have a look at the python logging page and the logging cookbook for some simple patterns on building a logger.

  • Other thoughts are: class Log() doesn't do what it says it is. It alerts, and passes a "download completed" message to the logger. I'd recommend separating the functionality there and renaming the class to what it actually does.

  • Move the code from Start() into the __main__ - you're not making it a class so there's no point on having that code separate.

  • For this (spelling error btw):

     if not self.location:
         self.loaction = os.getcwd()
    

you might want to just call self.GetSaveDir() as that's doing the prompting to get the value of the location to save, correct?

  • D.R.Y. here (also saving a log message to say "refer to the log is kind of redundant, right?):

     except Exception as err:
         AddLog(err)
         Log.error("Exception, please refer to log")
    
  • With:

     urlYT = self.getUrlE.get()
     if urlYT == "":
         Log.error("Please enter a url")
         return
    

Perhaps you could make this a loop (or check for 'exit' to abort) on the response. Something like (untested):

    while not (urlYT = self.getUrlE.get())
        Alerter.error("Please enter a URL or use 'exit' to abort.")
    if "exit" == urlYT:
        return
  • The function def CreateUI(self, master): is huge. You should be aiming for as few lines as possible in a function. I'd suggest refactoring each operation inside that function into their own separate functions so def CreateUI(self, master): becomes smaller. For example:

     def CreateUI(self, master):
         build_urls()
         build_file_format_getter()
         set_form_formats()
         set_location_buttons()
         set_logger()
         set_progress_bar()
         set_download_button()
    

The reason is, if you have a bug, you're not searching 50 lines for the cause. You only have to look at a specific function where the problem is.

  • Finally, the naming of your variables aren't PEP8. Hope this can help :-)
  • Definitely have a look at the python logging page and the logging cookbook for some simple patterns on building a logger.

  • Other thoughts are: class Log() doesn't do what it says it is. It alerts, and passes a "download completed" message to the logger. I'd recommend separating the functionality there and renaming the class to what it actually does.

  • Move the code from Start() into the __main__ - you're not making it a class so there's no point on having that code separate.

  • For this (spelling error btw):

     if not self.location:
         self.loaction = os.getcwd()
    

you might want to just call self.GetSaveDir() as that's doing the prompting to get the value of the location to save, correct?

  • D.R.Y. here (also saving a log message to say "refer to the log is kind of redundant, right?):

     except Exception as err:
         AddLog(err)
         Log.error("Exception, please refer to log")
    
  • With:

     urlYT = self.getUrlE.get()
     if urlYT == "":
         Log.error("Please enter a url")
         return
    

Perhaps you could make this a loop (or check for 'exit' to abort) on the response. Something like (untested):

    while not (urlYT = self.getUrlE.get())
        Alerter.error("Please enter a URL or use 'exit' to abort.")
    if "exit" == urlYT:
        return
  • The function def CreateUI(self, master): is huge. You should be aiming for as few lines as possible in a function. I'd suggest refactoring each operation inside that function into their own separate functions so def CreateUI(self, master): becomes smaller. For example:

     def CreateUI(self, master):
         build_urls()
         build_file_format_getter()
         set_form_formats()
         set_location_buttons()
         set_logger()
         set_progress_bar()
         set_download_button()
    

The reason is, if you have a bug, you're not searching 50 lines for the cause. You only have to look at a specific function where the problem is.

  • Finally, the naming of your variables aren't in line with PEP8. Hope this can help :-)
Source Link
C. Harley
  • 1.7k
  • 8
  • 8

  • Definitely have a look at the python logging page and the logging cookbook for some simple patterns on building a logger.

  • Other thoughts are: class Log() doesn't do what it says it is. It alerts, and passes a "download completed" message to the logger. I'd recommend separating the functionality there and renaming the class to what it actually does.

  • Move the code from Start() into the __main__ - you're not making it a class so there's no point on having that code separate.

  • For this (spelling error btw):

     if not self.location:
         self.loaction = os.getcwd()
    

you might want to just call self.GetSaveDir() as that's doing the prompting to get the value of the location to save, correct?

  • D.R.Y. here (also saving a log message to say "refer to the log is kind of redundant, right?):

     except Exception as err:
         AddLog(err)
         Log.error("Exception, please refer to log")
    
  • With:

     urlYT = self.getUrlE.get()
     if urlYT == "":
         Log.error("Please enter a url")
         return
    

Perhaps you could make this a loop (or check for 'exit' to abort) on the response. Something like (untested):

    while not (urlYT = self.getUrlE.get())
        Alerter.error("Please enter a URL or use 'exit' to abort.")
    if "exit" == urlYT:
        return
  • The function def CreateUI(self, master): is huge. You should be aiming for as few lines as possible in a function. I'd suggest refactoring each operation inside that function into their own separate functions so def CreateUI(self, master): becomes smaller. For example:

     def CreateUI(self, master):
         build_urls()
         build_file_format_getter()
         set_form_formats()
         set_location_buttons()
         set_logger()
         set_progress_bar()
         set_download_button()
    

The reason is, if you have a bug, you're not searching 50 lines for the cause. You only have to look at a specific function where the problem is.

  • Finally, the naming of your variables aren't PEP8. Hope this can help :-)