My first ever Python program, comments welcome

Ian Foote ian at feete.org
Sat Jul 21 15:34:48 EDT 2012


On 21/07/12 20:08, Lipska the Kat wrote:
> Greetings Pythoners
>
> A short while back I posted a message that described a task I had set 
> myself. I wanted to implement the following bash shell script in Python
>
> Here's the script
>
> sort -nr $1 | head -${2:-10}
>
> this script takes a filename and an optional number of lines to display
> and sorts the lines in numerical order, printing them to standard out.
> if no optional number of lines are input the script prints 10 lines
>
> Here's the file.
>
> 50    Parrots
> 12    Storage Jars
> 6    Lemon Currys
> 2    Pythons
> 14    Spam Fritters
> 23    Flying Circuses
> 1    Meaning Of Life
> 123    Holy Grails
> 76    Secret Policemans Balls
> 8    Something Completely Differents
> 12    Lives of Brian
> 49    Spatulas
>
>
> ... and here's my very first attempt at a Python program
> I'd be interested to know what you think, you can't hurt my feelings
> just be brutal (but fair). There is very little error checking as you 
> can see and I'm sure you can crash the program easily.
> 'Better' implementations most welcome
>
> #! /usr/bin/env python3.2
>
> import fileinput
> from sys import argv
> from operator import itemgetter
>
> l=[]
> t = tuple
What is this line supposed to do? If you're trying to make an empty 
tuple, you can write:
     t = ()
But I don't think this is needed at all.
> filename=argv[1]
> lineCount=10
>
> with fileinput.input(files=(filename)) as f:
>     for line in f:
>         t=(line.split('\t'))
>         t[0]=int(t[0])
>         l.append(t)
>     l=sorted(l, key=itemgetter(0))
>
>     try:
>         inCount = int(argv[2])
>         lineCount = inCount
I don't think you need to split this into two lines here.
         try:
             lineCount = int(argv[2])
should work.
>     except IndexError:
>         #just catch the error and continue
>         None
I would use pass instead of None here - I want to "do nothing" rather 
than create a None object.
>     for c in range(lineCount):
>         t=l[c]
>         print(t[0], t[1], sep='\t', end='')
>
> Thanks
>
> Lipska
>

My only other point is that you might find it helpful to use slightly 
more verbose names than l or t - its not immediately obvious to the 
reader what these are intended to represent.

Regards,
Ian



More information about the Python-list mailing list