i'm a python newbie & wrote my first script, can someone critique it?

Joel Goldstick joel.goldstick at gmail.com
Fri Jun 10 21:02:31 EDT 2016


On Fri, Jun 10, 2016 at 6:52 PM, mad scientist jr
<mad.scientist.jr at gmail.com> wrote:
> Is this group appropriate for that kind of thing?
> (If not sorry for posting this here.)
>
> So I wanted to start learning Python, and there is soooo much information online, which is a little overwhelming. I really learn best from doing, especially if it's something actually useful. I needed to create a bunch of empty folders, so I figured it was a good exercise to start learning Python.
> Now that it's done, I am wondering what kind of things I could do better.
> Here is the code:
>
> # WELCOME TO MY FIRST PYTHON SCRIPT!
> # FIRST OF ALL, IT ***WORKS***!!! YAY!
> # I AM A VBA AND JAVASCRIPT PROGRAMMER,
> # SO IT IS PROBABLY NOT VERY "PYTHONIC",
> # SO PLEASE FEEL FREE TO TEAR THE SCRIPT A NEW ONE
> # AFTER YOU ARE SHOCKED BY THIS BAD CODE,
> # ALL I ASK IS THAT IF YOU THINK SOMETHING IS BAD,
> # PLEASE POST AN EXAMPLE OF THE "CORRECT" WAY OF DOING IT
> # COMING FROM VBA, I KNOW A LITTLE OOP,
> # BUT NOT C++ C# JAVA STUFF LIKE "INTERFACES" AND "ABSTRACT BASE CLASSES"
> # SO IF YOU GET INTO SUCH TERMINOLOGY MY EYES MIGHT START TO GLAZE OVER
> # UNLESS YOU CARE TO EXPLAIN THAT TOO
>
>
Ditch the uppercase comments.
 ############################################################################################################################################################
> # GLOBAL VALUES
> sForPythonVersion="3"
> #sFolderPathTemplate = "c:\\myscripts\\MP3 Disc <iCount/>"
> sFolderPathTemplate = "c:\\temp\\MP3 Disc <iCount/>"
> iFromCount = 1
> iToCount = 250
> iCountWidth = 3
>
Python has guidelines for naming things.  Don't use camel case.  Use
lower case and put underscores between words.  Globals are a bad idea.
If you don't know why, they are even a worse idea
> ################################################################################################################################################################
> # SUPPORT FUNCTIONS
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def is_string(myVar): # is_string IS MORE READABLE THAN isinstance (PLAIN ENGLISH!)
>     #PYTHON 3 IS NOT LIKING THIS: return ( isinstance(myVar, str) or isinstance(myVar, unicode) )
>     #PYTHON 3 IS NOT LIKING THIS: return isinstance(myVar, basestr)
>     return isinstance(myVar, str)

I'm not sure why you want to test for type.
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> # THIS IS SOME SAMPLE FUNCTION FROM A BOOK I AM READING "PYTHON IN EASY STEPS"
> def strip_one_space(s):
>     if s.endswith(" "): s = s[:-1]
>     if s.startswith(" "): s = s[1:]
>     return s

This may be from some book, but it could be done:
s.strip()


>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def get_exact_python_version():
>     import sys
>     sVersion = ".".join(map(str, sys.version_info[:3]))
>     sVersion = sVersion.strip()
>     return sVersion
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> # TO DO: RETURN TO THE LEFT OF FIRST "."
> def get_python_version():
>     sVersion = get_exact_python_version()
>     return sVersion[0:1]
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> # CHECK PYTHON VERSION, IF IT'S WRONG THEN GRACEFULLY EXIT BEFORE IT BLOWS UP
> # (DAMN, PYTHON 2.x STILL COMPLAINS WITH SYNTAX ERRORS!!)
> # MAYBE THIS COULD STILL BE USEFUL FOR CHECKING THE SUB-VERSION, IN THAT CASE
> # TO DO: MORE GRANULAR CHECK, EG IF VERSION >= 3.5.0
> def exit_if_wrong_python_version(sRightVersion):
>     import os
>     sCurrentVersion = get_python_version()
>     if (sCurrentVersion != sRightVersion):
>         print("" +
>               "Wrong Python version (" +
>               sCurrentVersion +
>               "), this script should be run using Python " +
>               sRightVersion +
>               ".x. Exiting..."
>               )
>         os._exit(0)
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def get_script_filename():
>     import os
>     return os.path.basename(__file__)
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def get_timestamp():
>     import datetime
>     return datetime.datetime.strftime(datetime.datetime.now(), '%Y-%m-%d %H:%M:%S')
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
>
> def create_folder(sPath):
>     import os
>     import errno
>     #if not os.path.exists(directory):
>     #    os.makedirs(directory)
>     try:
>         os.makedirs(sPath, exist_ok=True)
>     except OSError as exception:
>         #if exception.errno != errno.EEXIST:
>         print("ERROR #" + str(exception.errno) + "IN makedirs")
>         raise
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
>
> def create_folders(sFolderPathTemplate:str="", iFromCount:int=1, iToCount:int=0, iCountWidth:int=0):
>     # MAKE SURE TEMPLATE'S A STRING. OH, IS THIS NOT "PYTHONIC"? WELL IT'S MORE READABLE
>     if is_string(sFolderPathTemplate) == False:
>         iFromCount = 1; iToCount = 0;
>
>     sName = ""
>     sCount = ""
>     iLoop = iFromCount
>     while (iLoop <= iToCount) and (len(sFolderPathTemplate) > 0):
>         sCount = "{0}".format(iLoop)
>         if (iCountWidth > 0):
>             sCount = sCount.zfill(iCountWidth)
>         sName = sFolderPathTemplate.replace("<iCount/>", sCount)
>         create_folder(sName)
>         iLoop = iLoop + 1
>
> ############################################################################################################################################################
> # MAIN LOGIC
>
> def main():
>     # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>     # MAKE SURE PYTHON VERSION IS CORRECT
>     exit_if_wrong_python_version(sForPythonVersion)
>     print("PYTHON VERSION (" + get_exact_python_version() + ") MATCHES REQUIRED VERSION (" + sForPythonVersion + ")")
>     #print("get_python_version       returns \"" + get_python_version()       + "\"")
>     #print("get_exact_python_version returns \"" + get_exact_python_version() + "\"")
>
>     # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>     # PRINT START TIMESTAMP
>     print("" + get_timestamp() + " " + get_script_filename() + " STARTED.")
>
>     # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>     # DO WHAT WE CAME TO DO
>     # IT'S PROBABLY BAD FORM TO REFERENCE GLOBAL VARIABLES INSIDE THE SCOPE OF A FUNCTION?
>     # BUT I WANT TO MAKE IT EASY TO CONFIGURE THE SCRIPT JUST BY CHANGING A COUPLE OF LINES
>     # AT THE TOP, WITHOUT HAVING TO SEARCH THROUGH THE CODE
>     create_folders(
>         sFolderPathTemplate,
>         iFromCount,
>         iToCount,
>         iCountWidth)
>
>     # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>     # NOW FINISH
>     print("" + get_timestamp() + " " + get_script_filename() + " FINISHED.")
>     #import os
>     #os._exit(0)
>
> ############################################################################################################################################################
>
> main()
> --
> https://mail.python.org/mailman/listinfo/python-list

I gave up.  I'm happy you want to learn python, and have such
enthusiasm, but you should practice some more
-- 
Joel Goldstick
http://joelgoldstick.com/blog
http://cc-baseballstats.info/stats/birthdays


More information about the Python-list mailing list