patternpythonMinor
Extracting RAR files in a directory
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
Note that I still use
Your function should also take
And then just call it with:
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:
Everything inside the
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.