python - Cleaner way to move renamed files when walking over directory -

i created script walk on directory , move music files music folder while renaming files using regular expression library check if files numbered (as find annoyance). script seems working fine, haven't encountered errors, wondering if there cleaner way this seems bit convoluted , unpythonic. (striving write cleaner code). cleaner i'm not asking rewrite entire code block(s), rather check_names function , it's implementation when moving files. if there other errors helpful pointed out below reason.

if terrible code apologize python "sins" may have committed have used os , regular expression modules few times before.

since learning how use these , have little exposure @ point, explanation of why go long way aid in understanding. os.path.join(....) attempt @ making cross-platform , not having hardcode paths. reason seems bit convuluted due runtime takes approximately 1-2 min following on 6-7 folders: extract compressed .zip archives original directory walk on these, rename files if needed, , moved them , go original directory , delete remnants of move, or normal runtime that's going on? (the archives ~100-300 mb)

the relevant functions are.

def check_names(dirpath, file_name):     check = false     new_name = none     first_check = re.compile("^\d\d - ").match(file_name)     second_check = re.compile("^\d\d ").match(file_name)     if first_check != none or second_check != none:        check = true        if first_check:           new_name = file_name[first_check.span()[1]:]           shutil.move(os.path.join(dirpath, file_name),                       os.path.join(dirpath, new_name))        else:           new_name = file_name[second_check.span()[1]:]           shutil.move(os.path.join(dirpath, file_name),                       os.path.join(dirpath, new_name))     return check, new_name  def move_music(source, destination, file_extension, sub_string):     source_dir = os.path.split(source)[-1]     dirpath, dirnames, filenames in os.walk(source):         a_file in filenames:             if (a_file.endswith(file_extension) , sub_string in a_file):                 check = check_names(dirpath, a_file)                 dir_name = os.path.split(dirpath)[-1]                 if dir_name != source_dir:                     check_folders(destination, dir_name)                     if os.path.join(source, dir_name) not in copied_directories:                           copied_directories.append(os.path.join(source, dir_name))                     shutil.move(os.path.join(dirpath, a_file if not check[0] else check[1]),                                 os.path.join(destination , dir_name))                  else:                     shutil.move(os.path.join(dirpath, a_file if not check[0] else check[1]), destination) 

first things first:

you can break tasks , increase readability dramatically.

old:

if first_check:   new_name = file_name[first_check.span()[1]:]   shutil.move(os.path.join(dirpath, file_name),               os.path.join(dirpath, new_name)) else:    new_name = file_name[second_check.span()[1]:]    shutil.move(os.path.join(dirpath, file_name),                os.path.join(dirpath, new_name)) 

new:

if first_check:     new_name = file_name[first_check.span()[1]:] else:     new_name = file_name[second_check.span()[1]:] shutil.move(os.path.join(dirpath, file_name),             os.path.join(dirpath, new_name)) 

you solely changing file_name new_name in if/else statement, not need have long function call inside if/then statement.

next, define globals:

first_check = re.compile("^\d\d - ") second_check = re.compile("^\d\d ") 

next, regex matches automatically trigger if statement, can remove:

if first_check != none: 

and replace with:

if first_check: 

next restructuring code: second function long , unwieldy. break up. change move_music 2 functions:

def move_music(source_dir, destination, dirpath, a_file):     check = check_names(dirpath, a_file)     dir_name = os.path.split(dirpath)[-1]     if dir_name != source_dir:         check_folders(destination, dir_name)         if os.path.join(source, dir_name) not in copied_directories:               copied_directories.append(os.path.join(source, dir_name))         shutil.move(os.path.join(dirpath, a_file if not check[0] else check[1]),                     os.path.join(destination , dir_name))      else:         shutil.move(os.path.join(dirpath, a_file if not check[0] else check[1]), destination)  def check_move(source, destination, file_extension, sub_string):     source_dir = os.path.split(source)[-1]     dirpath, dirnames, filenames in os.walk(source):         a_file in filenames:             if (a_file.endswith(file_extension) , sub_string in a_file):                 move_music(source_dir, destination, dirpath, a_file) 

next, code length long per line: limit 72 characters doc statements, 79 actual code. enhances readability: how else can coder view code on split screen text editor?

you can partially breaking down redundant statements , further restructuring code:

def get_old_file(check, a_file):     if not check[0]:         old_file = a_file     else:         old_file = check[1]     return old_file 

here's end code: still need work! comment , add documentation strings.

edited, @leewangzhong, correctly states there regex caching, , since using 2 regexes, not need worry explicitly compiling.

def check_names(dirpath, file_name):     check = false     new_name = none     first_check = re.match("^\d\d - ", file_name)     second_check = re.match("^\d\d ", file_name)     if first_check or second_check:         check = true         if first_check:             new_name = file_name[first_check.span()[1]:]         else:             new_name = file_name[second_check.span()[1]:]         old_path = os.path.join(dirpath, file_name)         new_path = os.path.join(dirpath, new_name)         shutil.move(old_path, new_path)     return check, new_name   def get_old_file(check, a_file):     if not check[0]:         old_file = a_file     else:         old_file = check[1]     return old_file  def move_music(source_dir, destination, dirpath, a_file):     check = check_names(dirpath, a_file)     dir_name = os.path.split(dirpath)[-1]     if dir_name != source_dir:         check_folders(destination, dir_name)         if os.path.join(source, dir_name) not in copied_directories:                 path = os.path.join(source, dir_name)                 copied_directories.append(path)         old_file = get_old_file(check, a_file)         old_path = os.path.join(dirpath, old_file)         new_path = os.path.join(destination , dir_name)         shutil.move(old_path, new_path)      else:         old_file = get_old_file(check, a_file)         old_path = os.path.join(dirpath, old_file)         shutil.move(old_path, destination)  def check_move(source, destination, file_extension, sub_string):     source_dir = os.path.split(source)[-1]     dirpath, dirnames, filenames in os.walk(source):         a_file in filenames:             if (a_file.endswith(file_extension)                  , sub_string in a_file):                 move_music(source_dir, destination, dirpath, a_file)