Can you check this short code? Thanks

This is the place for queries that don't fit in any of the other categories.

Can you check this short code? Thanks

Postby portia » Sun Jun 30, 2013 6:06 pm

Here's the code I wrote:

Code: Select all
#!/usr/bin/python3

import zipfile
import re

zfile = zipfile.ZipFile('ziparchive.zip', 'r')
all_indexes = []

file_contents = zfile.read('90052.txt')

while True:
        match = re.findall(b'\d+', file_contents)

        # break if there are no numbers so the length of the 'match' list is 0
        if len(match) == 0:
            break

        index_file = match[0].decode("utf-8")
        next_file = index_file + ".txt"
        all_indexes.append(next_file)
        file_contents = zfile.read(next_file)
        print(file_contents)

print('.'.join([zfile.getinfo(i).comment.decode("utf-8") for i in all_indexes]))


Could you have a look at it and suggest any improvements? I mean it works okay but I'm far from expert and not sure if I'm not doing something in a clumsy and/or not recommended way.

Also, it it really necessary to decode it from bytes to strings in 2 places? Would not it possible to do it once?
Thank you.
portia
 
Posts: 17
Joined: Sun Apr 14, 2013 10:03 pm

Re: Can you check this short code? Thanks

Postby micseydel » Sun Jun 30, 2013 8:25 pm

The only thing here that I would definitely do differently is replace the list comprehension at the end with a generator comprehension.
portia wrote:
Code: Select all
print('.'.join([zfile.getinfo(i).comment.decode("utf-8") for i in all_indexes]))

would become
Code: Select all
print('.'.join(zfile.getinfo(i).comment.decode("utf-8") for i in all_indexes))

It's just less characters, and potentially uses less memory.
Join the #python-forum IRC channel on irc.freenode.net!
User avatar
micseydel
 
Posts: 1118
Joined: Tue Feb 12, 2013 2:18 am
Location: Mountain View, CA

Re: Can you check this short code? Thanks

Postby portia » Sun Jun 30, 2013 8:41 pm

Ok, thanks. I'll modify it. I haven't heard of generator comprehensions. Good... a new thing to learn.
portia
 
Posts: 17
Joined: Sun Apr 14, 2013 10:03 pm

Re: Can you check this short code? Thanks

Postby metulburr » Mon Jul 01, 2013 12:17 am

this is getting really picky, but since you asked for improvements:
Code: Select all
while True:
        match = re.findall(b'\d+', file_contents)

        # break if there are no numbers so the length of the 'match' list is 0
        if len(match) == 0:
            break

        index_file = match[0].decode("utf-8")
        next_file = index_file + ".txt"
        all_indexes.append(next_file)
        file_contents = zfile.read(next_file)
        print(file_contents)

You have the while loop block indentated at 8 spaces and the if condition inside indented at 4 spaces. Of course that just might be a typo on posting it here, not sure.
New Users, Read This
OS Ubuntu 14.04, Arch Linux, Gentoo, Windows 7/8
https://github.com/metulburr
steam
User avatar
metulburr
 
Posts: 1312
Joined: Thu Feb 07, 2013 4:47 pm
Location: Elmira, NY

Re: Can you check this short code? Thanks

Postby Mekire » Mon Jul 01, 2013 2:15 am

Also being nitpicky; you could replace this:
Code: Select all
if len(match) == 0:
    break
with this:
Code: Select all
if not match:
    break

-Mek
User avatar
Mekire
 
Posts: 982
Joined: Thu Feb 07, 2013 11:33 pm
Location: Amakusa, Japan

Re: Can you check this short code? Thanks

Postby portia » Mon Jul 01, 2013 5:51 am

Thank you. Yes. that's what I meant:)

I didn't notice. It was 8 spaces!

I should have remembered about the if statement. I read about "the python way" somewhere last week.
portia
 
Posts: 17
Joined: Sun Apr 14, 2013 10:03 pm

Re: Can you check this short code? Thanks

Postby ochichinyezaboombwa » Mon Jul 01, 2013 2:50 pm

I have no idea what is this script about, but you do realize that the result of
Code: Select all
file_contents = zfile.read('90052.txt')
is never used to
Code: Select all
print(file_contents)
?
Also, do you particularly like that the
Code: Select all
print(file_contents)
statement occurs twice?
ochichinyezaboombwa
 
Posts: 200
Joined: Tue Jun 04, 2013 7:53 pm

Re: Can you check this short code? Thanks

Postby portia » Mon Jul 01, 2013 7:14 pm

ochichinyezaboombwa wrote:I have no idea what is this script about, but you do realize that the result of
Code: Select all
file_contents = zfile.read('90052.txt')
is never used to
Code: Select all
print(file_contents)
?

Yes, it's not meant to be.
ochichinyezaboombwa wrote:Also, do you particularly like that the
Code: Select all
print(file_contents)

statement occurs twice?

Sorry, not following you.
portia
 
Posts: 17
Joined: Sun Apr 14, 2013 10:03 pm


Return to General Coding Help

Who is online

Users browsing this forum: No registered users and 2 guests