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.
Due to the reasons discussed here we will be moving to python-forum.io on October 1, 2016.

This forum will be locked down and no one will be able to post/edit/create threads, etc. here from thereafter. Please create an account at the new site to continue discussion.
User avatar
micseydel
 
Posts: 3000
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.
we will be moving to python-forum.io on October 1 2016
more details here
User avatar
metulburr
 
Posts: 2243
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
New Users, Read This
  • Use code tags when posting code.
  • Include any errors with your post (in code tags).
  • Describe your problem; not your chosen solution.
  • Make examples the minimum length to demonstrate your issue.
User avatar
Mekire
 
Posts: 1710
Joined: Thu Feb 07, 2013 11:33 pm
Location: Tucson, Arizona

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: 203
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 3 guests