My original code is here. I made some changes to it based on your suggestions, but then I lost my flash drive and had to redo it. I have a suspicion that my second version was better than this one, so could you all help me out one more time?

import os, shutil, zipfile

def subtractname(path):
    newpath = str()
    path = path.split("\\")
    for x in range(len(path) - 1):
        newpath += (path[x] + "\\")
    newpath += path[len(path) - 1]
    return newpath

def pathtoname(path):
    path = path.split("\\")
    newpath = path[len(path) - 1]
    newpath = newpath[:-4]

def makepath(path):
    except FileExistsError:

def getscds(targetfolder):
    global scdlist
    filelist = os.listdir(targetfolder)
    scdlist = []
    for file in filelist:
        if '.scd' in file:
            scdlist.append(os.path.join(targetfolder, file))
    return scdlist

def unzip(file, target):
    modscd = zipfile.ZipFile(file)

def checkdirs(target):
    isfile = 0
    targetlist = os.listdir(target)
    for files in targetlist:
        if not os.path.isfile(files):
            isfile = isfile + 1
    if isfile == len(os.listdir(target)):
        return False
        return True

def install(folderlist):
    newpathlist = []
    for folder in folderlist:
        pathlist = getscds(folder)
        for path in pathlist:
           newpath = os.path.join(subtractname(path), ("f_" + pathtoname(path)))
           unzip(path, newpath)
           if checkdirs(newpath):
            shutil.copy(path, r"C:\Program Files (x86)\Steam\steamapps\common\supreme commander 2\gamedata")
    return newpathlist

a = input()
b = [a]

A note: this code SHOULD actually work if you run it, but I haven't incorporated error handling yet.

I know that the last three lines can be condensed into one, but aside from that is there anything that could be improved here? I'm not really looking for naming conventions or comments, but I'm fine with it if you want to throw them in along with some more practical suggestions.

EDIT: I just tried running this on a friend's computer, and it may not work after all. Ideas?


I'll leave an in-depth analysis to the true Python experts, but here are just some quick notes:

Variable and function names: As far as I'm aware, variable and function names should have words/elements separated by underscores (e.g., check_dirs, is_file)

main() function: Instead of having your functionality on the first/default level of indentation, it's good form to wrap it in a def main() and call it via:

if __name__ == '__main__':

Return values: You can simply return a bool directly rather than having an if/else construction. e.g.,

return isfile != len(os.listdir(target))


if isfile == len(os.listdir(target)):
    return False
    return True

Use of global: In my experience, any time I have to use global some_var, it's indicative of bad design. You should either be using local variables, function arguments, or the constant should be well-defined and not hidden by any local fields.

  • \$\begingroup\$ Yeah, the global thing was left over from version 1. I'll take it out as it doesn't seem to serve a purpose anymore. \$\endgroup\$
    – KnightOfNi
    Apr 14 '14 at 16:36

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