tags:

views:

155

answers:

5

I have this code:

def GetSteamAccts(): #Get list of steam logins on this computer.
    templist = []
    Steamapp_Folders = ["C:\\Program Files (x86)\\Steam\\steamapps\\", "C:\\Program Files\\Steam\\steamapps\\"] #Check both of these directories.
    for SF_i in range(len(Steamapp_Folders)):
        if os.path.exists(Steamapp_Folders[SF_i]): #If the directory even exists...
            Steam_AppDir_Items = os.listdir(Steamapp_Folders[SF_i]) #List items under steam install directory.
            for S_AD_i in range(len(Steam_AppDir_Items)): #Make sure the user doesn't have any files in here...
                if os.path.isdir(Steamapp_Folders + Steam_AppDir_Items[S_AD_i]): #If our path is a directory...
                    templist.append(Steam_AppDir_Items[S_AD_i])  #Add it to our list of logins.
                                                                 #(If some idiot puts extra folders in here,
                                                                 #it's their own damn fault when it shows on the list.)
    return templist #Return a (not so) properly filtered list of steam logins.

My problem is, it looks AWFULLY ugly to me. I made a list of 2 paths (only 1 will ever exist), loop over those paths, then I have to get a list of the items in those paths, and then traverse those and filter out non-directories from that in order to get a pseudo-list of steam logins on a users computer. (Basically just getting a list of any existing directories (directories only!) under either of those 2 paths)

Is there a shorter way of doing this (Other than condensing for loops into single lines?)?

I would much rather be given an english-worded solution so I can put it together myself; rather than code. It's the only way I'll truly learn the proper way. Even a nice little hint or excerpt so I can figure it out on my own would be nice.

And: Do lists in for loops always have to be traversed like:

for x in range(len(somelist)):

or is there something shorter than using range(len( ?

+9  A: 
for i in range(len(somelist)):
   something( somelist[i] )

should be written as

for x in somelist: 
    something( x )

Also you can write everything much shorter:

def GetSteamAccts():
    Steamapp_Folders = [f for f in ("C:\\Program Files (x86)\\Steam\\steamapps\\", 
                                    "C:\\Program Files\\Steam\\steamapps\\") 
                          if os.path.isdir(f)]
    return [os.path.join(root, folder) 
                    for root in Steamapp_Folders 
                    for folder in os.listdir(root) 
                    if os.path.isdir( os.path.join(root, folder)) ]

This looks cleaner and actually does what you wanted: ;-)

def subfoldernames( root ):
    for folder in os.listdir(root):
        path = os.path.join(root, folder)
        if os.path.isdir(path):
            yield folder # just the name, not the path

# same, just shorter:
def subfoldernames( root ):
    # this returns a generator, written as a generator expression
    return ( folder for folder in os.listdir( root ) 
                    if os.path.isdir(os.path.join( root, folder )) )

def GetSteamAccts():
        Steamapp_Folders = ("C:\\Program Files (x86)\\Steam\\steamapps\\", 
                                             "C:\\Program Files\\Steam\\steamapps\\")
        for folder in Steamapp_Folders:
            if os.path.isdir(folder):
                # only the subfolders of the first path that exists are returned
                return list(subfoldernames( folder )) 
THC4k
Although this uses a lengthy list comprehension (two loops and a condition), note that with proper indenting/line breaks it's still readable. "Condensing for loops into single lines" doesn't have to mean it's on a single line, or dense and ugly!
Ipsquiggle
no matter how many lines you spread it, it will remain dense, ugly and unreadable nonetheless.
SilentGhost
Thanks THC; this is elegant and concise enough that I understand what's going on (except I'm gonna need to look more at "f for f in"), but at least I understand what it's doing. It's basically stripping the list of paths that don't exist before doing the next section of code. Thanks again, I'll be playing with this ;)
ThantiK
I write anything I can as a list comprehension or as a series of generators. I think it's much simpler since you have all the code that affects the list in one place.
THC4k
I used your previous example because it's much cleaner in my eyes and splitting everything into different functions like that (when I don't need to re-use the functionality) makes things more scattered to me. I understood the first example well enough that I rewrote it to do what I needed (and to help make myself learn it) ;)I guess compactness is more important to me than the re-usability of the code.
ThantiK
+5  A: 

And: Do lists in for loops always have to be traversed like:

for x in range(len(somelist)): or is there something shorter than using range(len( ?

Sure there is, if you want to access the item only and you are not interested in its index you could do this:

for x in somelist:

If you want the index as well you could do this:

for index, x in enumerate(somelist):
Nadia Alramli
+4  A: 

You should lose the idea that a for loop always iterates over numbers from zero to something, like in other languages. You're just traversing lists, so use the for loop like this:

def GetSteamAccts():
    templist = []
    Steamapp_Folders = ["C:\\Program Files (x86)\\Steam\\steamapps\\", "C:\\Program Files\\Steam\\steamapps\\"] #Check both of these directories.
    for steamapp_folder in Steamapp_Folders:
        if os.path.exists(steamapp_folder):
            for steam_appDir_item in os.listdir(steamapp_folder):
                if os.path.isdir(os.path.join(steamapp_folder, steam_appDir_item)):
                    templist.append(steam_appDir_item)
    return templist

In Python, a for loop goes over anything that can be iterated. For those few times where you actually need the numbers (and only the numbers), there's range. For those times where you need the numbers and the items, use this:

for number, item in enumerate(my_list):

When combining fors and ifs, you should also take a look at generator expressions and list comprehensions (see the docs).

balpha
Thanks. Unfortunately I worked with like...VB4 back when I was a young teenager and then out of the blue stopped programming. A lot of very messed up stuff sticks with me and I'm trying to unlearn a lot in order to facilitate doing things smoother.
ThantiK
@ThantiK: Stack Overflow is just the right place for that :-) And, as for learning Python, you'll be appreciating many things that Py.thon does for making your life easier than VB4
balpha
Program in Python long enough and it will be the other way aroud. When you work in some other language having to set up an integer variable to count how many times you've been through a loop will seem like a huge pain.
Chris Upchurch
A: 

I'd suggest something like this:

def check_subfolder(*args):
    for folder in args:
         for root, dirs, _ in os.walk(folder):
              return [os.path.join(root, i) for i in dirs]  # since only 1 will ever exist

it's just saves on all those if statements. Of course, first for loop could be moved out of the function, because it's not essential for the logic of the code.

edit: updated to return subdirs of the first existing directory, as per question.

SilentGhost
+2  A: 

As others have mentioned, removing the assumption about integer indexes removes a large amount of the complexity.

Going a bit further, you can replace patterns where you build up a list by appending in a for loop with a list comprehension. How much (or whether) these improve readability is debatable once you get into several layers of nesting, but they do make the code much more concise:

def GetSteamAccts():
    Steamapp_Folders = ["C:\\Program Files (x86)\\Steam\\steamapps\\", "C:\\Program Files\\Steam\\steamapps\\"] #Check both of these  directories.
    return [ item for folder in Steamapp_Folders if os.path.exists(folder)
                  for item in os.listdir(folder) if os.path.isdir(os.path.join(folder, item)) ]
Brian