Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have written a Python function which matches all files in the current directory with a list of extension names. It is working correctly.

import os, sys, time, re, stat

def matchextname(extnames, filename):
    # Need to build the regular expression from the list
    myregstring = ""
    for index in range(len(extnames)):
        # r1 union r2 and  so on operator is pipe(|)
        # $ is to match from the end
        if index <  len(extnames) - 1:
            myregstring =  myregstring + extnames[index] + '$' + '|'
        else:
            myregstring = myregstring + extnames[index] + '$'
    # getting regexobject
    myregexobj = re.compile(myregstring)
    # Now search
    searchstat = myregexobj.search(filename)
    if searchstat:
        print 'Regex', filename

It is called like this:

if __name__ == '__main__':
    fileextensions = ['\.doc', '\.o', '\.out', '\.c', '\.h']
    try:
        currentdir = os.getcwd()
    except OSError:
        print 'Error occured while getting current directory'
        sys.exit(1)
    for myfiles in os.listdir(currentdir):
        matchextname(fileextensions, myfiles)

Could you please review the code and suggest if there is any better way of doing this, or share any other comments related to errors/exception handling which are missing - or anything else in terms of logic?

share|improve this question

2 Answers 2

up vote 3 down vote accepted

I think the way to code this, that most clearly indicates what you are doing, is to use os.path.splitext to get the extension, and then look it up in a set of extensions:

import os.path
extensions = set('.doc .o .out .c .h'.split())
_, ext = os.path.splitext(filename)
if ext in extensions:
    print(filename)

A couple of other comments on your code:

  1. There's no need to catch the OSError if all you're going to do is print a message and exit. (This will happen in any case if the exception is uncaught, so why go to the extra trouble?)

  2. If you actually want to build up a regular expression, then do it using str.join. This avoids the need to have a special case at the end:

    extensions = r'\.doc \.o \.out \.c \.h'.split()
    myregexobj = re.compile('(?:{})$'.format('|'.join(extensions)))
    

    (Whenever you find yourself writing a loop over the indexes to a sequence, then you should think about rewriting it to loop over the elements of the sequence instead: this nearly always results in clearer and shorter code.)

  3. If you want to build a regular expression that exactly matches a string literal, you should use re.escape to escape the special characters, instead of escaping each one by hand. For example:

    extensions = '.doc .o .out .c .h'.split()
    myregexobj = re.compile('(?:{})$'.format('|'.join(map(re.escape, extensions))))
    
share|improve this answer
    
Thanks Gareth. Your explanation and all the methods that you have shared are excellent for learning. These really helped me to understand many new and better techniques. Thanks again. –  Tanmoy Nov 28 '12 at 18:18

.endswith() accepts a tuple:

#!usr/bin/env python
import os

fileextensions = ('.doc', '.o', '.out', '.c', '.h')
for filename in os.listdir(os.curdir):
    if filename.endswith(fileextensions):
       print(filename)
share|improve this answer
    
Sebastian. Thanks a lot for your help. This is quite a nice approach, which you have shown –  Tanmoy Nov 28 '12 at 18:15

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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