Skip to content
Advertisement

Is there a more efficient way instead of multiple FOR loops and IF statements?

I have the below code snippet:

for item1 in filesName :
    for item2 in finalName :
        if item1.split('.',1)[0].split('--', 1)[0] == item2.split('.',1)[0].split('--', 1)[0] :
            for eachFile in os.listdir(src) :
                if eachFile == item1 :
                
                    # rename files
                    os.rename(os.path.join(src, item1), os.path.join(src, item2))

                    # move files
                    shutil.move(os.path.join(src, item2), os.path.join(dst, item2))
            
                else :
                    logger.error(error_message)
                    raise ValueError(error_message)

FilesName = ['01-01-2001 Active File Name.xlsx', '01-01-2001 Inactive File Name.xlsx']

FinalName = ['01-01-2001 Active File Name--CORRECTION1.xlsx']

What it intends to do is rename an existing file in case of a correction – for example, I have a file called ‘File Name.xlsx’, but I want to rename it to ‘File Name –CORRECTION1.xlsx’ – it matches the file to each other and renames it.

The code works as intended, however my limited experience tells me that I’m using too many FOR loops and IF statements, and there’s probably a better way [performance wise] to do it.

So what’s my question – is there a better solution?

Advertisement

Answer

FilesName = ['01-01-2001 Active File Name.xlsx', '01-01-2001 Inactive File Name.xlsx']
FinalName = ['01-01-2001 Active File Name--CORRECTION1.xlsx']

# Helper function to clean up the splitting
get_base_name = lambda x: x.split('.',1)[0].split('--', 1)[0]

# Remove the two first loops by checking for base name matches
# using set intersection
filesName_base = {get_base_name(x) for x in FilesName}
finalName_base = {get_base_name(x) for x in FinalName}
files_to_change_base = filesName_base.intersection(finalName_base)

# Potential candidates of files to be changed
files_to_change = [x for x in FilesName if get_base_name(x) in files_to_change_base]

# No need to call this on each iteration of the inner loop
files_in_dir = set(os.listdir(src))

for file_to_change in files_to_change:
    if file_to_change in files_in_dir:
        # rename files
        os.rename(os.path.join(src, item1), os.path.join(src, item2))
        # move files
        shutil.move(os.path.join(src, item2), os.path.join(dst, item2))

    else :
        logger.error(error_message)
        raise ValueError(error_message)

Edit: Removed another for loop by just looping once and checking if the files are in the directory. files_in_dir is moved into a set because set membership is a O(1) operation as opposed to O(n).

User contributions licensed under: CC BY-SA
7 People found this is helpful
Advertisement