Ggtcf

So I am new to python and I have a function that I need to break into two parts. Previously it was one function but after some advice from someone that knows way more than me, I was given the tip that my function did too much, and I need to break it down to two separate things; so here I am.

Below is the code broken into two parts.

I am wondering do I have to mention the pathlist in both functions?

What this is supposed to do is check if the files exist and then if they do then run the second function to remove the actual directories.

def check_directory(pathslist):
 for path in pathslist:
 if os.path.exists(path) and os.path.isdir(path):
 remove_directory(pathslist)

dirs_to_delete = [
 'C:MyDirectoryPath1',
 'C:MyDirectoryPath2',
 'C:MyDirectoryPath3'

 ]

def remove_directory(pathslist):
 for path in pathslist:
 if os.path.exists(path) and os.path.isdir(path):
 shutil.rmtree(path)
 print(colored('Found ' + path + ' removing', 'green'))

Not exactly. If you are passing in the entire pathslist to remove_directory, you're going to try to remove each one whether or not it exists, making your check_directory function unnecessary. I think what you mean is in your check_directory function to only pass the path that exists in to remove_directory:

def check_directory(pathslist):
 for path in pathslist:
 if os.path.exists(path) and os.path.isdir(path):
 remove_directory(path)

dirs_to_delete = [
 'C:MyDirectoryPath1',
 'C:MyDirectoryPath2',
 'C:MyDirectoryPath3'

 ]


def remove_directory(path):
 shutil.rmtree(path)
 print(colored('Found ' + path + ' removing', 'green'))

You may want to try writing a comment for every function you write describing what it does. The second you use the word "and" or an additional verb, that's a hint that you may be better off splitting the function into multiple parts (that's just a rule of thumb, not an absolute). In addition, you want to avoid repeating code--if you have the same lines of code in two separate functions, that's another hint that you need to rethink your design.

Edit: As pointed out in the comments, the way you've written it means that calling check_directory will remove the directory if it exists. It seems reasonable to expect that someone would call check_directory for reasons other than wanting to remove it, and you'd be better off having remove_directory call check_directory rather than the other way around:

 def check_directory(path):
 # returns true if path is an existing directory
 return os.path.exists(path) and os.path.isdir(path)

 def remove_directory(pathlist):
 for path in pathlist:
 if check_directory(path):
 shutil.rmtree(path)
 print(colored('Found ' + path + ' removing', 'green'))