Cleaning up nested for loops in python
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 l开发者_如何学Pythonist 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( ?
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 ))
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):
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 for
s and if
s, you should also take a look at generator expressions and list comprehensions (see the docs).
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)) ]
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.
精彩评论