2
\$\begingroup\$

This is a simple script for moving my MP3 files from one directory to another. I'd like to know if there is a more efficient way to handle my task in the script. I analyzed the algorithm to be \$O(n^2)\$.

import os
import shutil
import re

files = os.listdir(os.path.dirname('/home/rich/Downloads/'))

regex = r"(.mp3)"

for f in files:
    if (re.search(regex, f)):
        f = os.path.join('/home/rich/Downloads/', f)
        shutil.move(f, '/home/rich/Music/Songs/')
\$\endgroup\$
5
  • \$\begingroup\$ You could do a benchmark and compare the solution against using fnmatch. However, it hardly seems a performance-critical script. \$\endgroup\$
    – D. Jurcau
    Jun 11, 2016 at 14:29
  • \$\begingroup\$ How did you arrive at O(n^2)? You only have one loop. \$\endgroup\$
    – D. Jurcau
    Jun 11, 2016 at 14:58
  • \$\begingroup\$ I thought that the search for ".mp3" for each file with regex took O(n) time as well. \$\endgroup\$
    – mitchr06
    Jun 11, 2016 at 15:49
  • 2
    \$\begingroup\$ Performing a constant (\$k\$) number of operations on each file brings to complexity to \$O(kn)\$. For it to have been \$O(n^2)\$, you would have needed to loop through all files each and every time you needed to perform an operation on one of them. \$\endgroup\$
    – D. Jurcau
    Jun 11, 2016 at 16:24
  • \$\begingroup\$ hmm. I miss understood that then. I didn't think that the regex search was a constant operation. Thanks. \$\endgroup\$
    – mitchr06
    Jun 11, 2016 at 18:33

1 Answer 1

3
\$\begingroup\$
  1. Constants like '/home/rich/Downloads/' could be made into variables, for example:

    src_dir = '/home/rich/Downloads/'
    dest_dir = '/home/rich/Music/Songs/'
    

    This would make it clear what they mean, and would avoid duplication if you need to use a constant more than once.

  2. There are three problem in the regular expression.

    First, the parentheses are unnecessary — they capture whatever they match for future use, but there's no future use here.

    Second, the dot has a special meaning in regular expressions — it matches any character. If you want to match a literal dot, then you need to escape it.

    Third, the regular expression is not anchored and that means it can match anywhere in the string. For example, it will match the filename "not.actually.an.mp3.wav".

    The regular expression should be r"\.mp3$".

  3. There's no need for a regular expression at all, you could just call os.path.splitext to get the extension:

    for f in files:
        _, ext = os.path.splitext(f)
        if ext == '.mp3':
            shutil.move(f, dest_dir)
    
  4. In fact there's no need to do that, because you could use glob.glob instead:

    for f in glob.glob(os.path.join(src_dir, '*.mp3')):
        shutil.move(f, dest_dir)
    
  5. It seems like overkill to use Python for this, because it would be a one-liner in shell:

    mv ~/Downloads/*.mp3 ~/Music/Songs/
    
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.