I wrote this little snippet which moves files from several subfolders to their parent folder. Since the snippet uses different nested for loops I was wondering if it could be refactored to a cleaner and more maintainable solution? I tried extracting the different for loops to separate functions, but they rely on the same variables so I couldn't get this to work.
import os
import shutil
import sys
currentDirectory = os.getcwd()
def changeStructure(currentFolder):
parentDirectory = currentDirectory + "/" + currentFolder
for folder in os.listdir(parentDirectory):
folderPath = parentDirectory + "/" + folder
if os.path.isdir(folderPath):
for file in os.listdir(folderPath):
if not file.startswith('.'):
shutil.move(folderPath + "/" + file, parentDirectory)
shutil.rmtree(folderPath)
print("All folders removed")
if __name__ == '__main__':
globals()[sys.argv[1]](sys.argv[2])
1 Answer 1
You should replace os
with pathlib
equivalents since it has a nicer, object-oriented interface.
currentDirectory
(which should be called current_directory
) should not be a global. In a situation where you load the module, change the working directory and then call changeStructure
this will do the wrong thing.
Add PEP484 type hints.
Consider inverting your if isdir
condition to be able to de-dent the rest of the loop.
Your use of globals()
is evil. It allows people invoking your application unfettered access to anything that you import and anything that you define on the global namespace, even if it starts with underscores. Do basically anything that isn't this. Since this is a single-method program, in this case just pass the first argument directly to your subroutine.
Suggested
Not tested because I don't want to mangle my filesystem.
import shutil
import sys
from pathlib import Path
def change_structure(current_folder: str) -> None:
parent_dir = Path.cwd() / current_folder
for folder in parent_dir.iterdir():
if not folder.is_dir():
continue
for child in folder.iterdir():
if not child.name.startswith('.'):
child.rename(parent_dir / child.name)
shutil.rmtree(folder)
print("All folders removed")
if __name__ == '__main__':
change_structure(sys.argv[1])