HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Extracting RAR files in a directory

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rardirectoryfilesextracting

Problem

I just started programming, and something is telling me that this working function looks to big for what it does. Can you give a better version of the code so I can understand how to code faster, shorter code ?

import os, rarfile

xpath= "/root/tree/def/"

dpath="/root/tree/down"

def unrar():
        for rf in os.listdir(dpath):
            f = os.path.join(dpath,rf)
            o = rarfile.RarFile(f)
            for u in o.infolist():
             print (u.filename, u.file_size)
             o.extractall(xpath)

Solution

You should use more descriptive names than f, o, u and rf. It makes your code harder to read, even if it's all quite small and contained.

def unrar():
    for rar in os.listdir(dpath):
        filepath = os.path.join(dpath, rar)
        opened_rar = rarfile.RarFile(filepath)
        for f in opened_rar.infolist():
            print (f.filename, f.file_size)
        opened_rar.extractall(xpath)


Note that I still use f for the files you iterate over in the opened_rar, because single letter variables are occasionally ok. Especially in limited contexts, where they're commonly used (f will often refer to a file when iterating). But using multiple ones together can easily be confusing and less readable.

Your function should also take xpath and dpath as parameters, rather than using constants. This makes it easier to reuse the function and have it run on different files or in different places, rather than having to manually change the constant values. It's also easy to change:

def unrar(dpath, xpath):


And then just call it with:

unrar(dpath, xpath)


Also Python has a great tool called context managers. These will basically ensure that your file is always closed no matter what happens. Every time you open a file with Python it needs to be closed again. And you actually never closed yours, so it's better practice to use a context manager, like this:

def unrar(dpath, xpath):
    for rar in os.listdir(dpath):
        filepath = os.path.join(dpath, rar)
        with rarfile.RarFile(filepath) as opened_rar:
            for f in opened_rar.infolist():
                print (f.filename, f.file_size)
            opened_rar.extractall(xpath)


Everything inside the with block is now what happens with the opened_rar file. Once you exit that block, the file is always closed. Crucially, this holds true even if an error causes your script to end prematurely. I haven't worked with rar files, but I can tell you that when using zip files you can lose your data by having errors interrupt your script. A context manager prevents this data loss, making it safer.

Code Snippets

def unrar():
    for rar in os.listdir(dpath):
        filepath = os.path.join(dpath, rar)
        opened_rar = rarfile.RarFile(filepath)
        for f in opened_rar.infolist():
            print (f.filename, f.file_size)
        opened_rar.extractall(xpath)
def unrar(dpath, xpath):
unrar(dpath, xpath)
def unrar(dpath, xpath):
    for rar in os.listdir(dpath):
        filepath = os.path.join(dpath, rar)
        with rarfile.RarFile(filepath) as opened_rar:
            for f in opened_rar.infolist():
                print (f.filename, f.file_size)
            opened_rar.extractall(xpath)

Context

StackExchange Code Review Q#116843, answer score: 4

Revisions (0)

No revisions yet.