Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Hi it's from my first program in Python. How I can make it better?

from collections import namedtuple
Point = namedtuple('Point ', ['x', 'y'])
def read():
    ins = open("PATH_TO_FILE", "r")
    array = []
    first = True
    expected_length = 0
    for line in ins:
        if first:
            expected_length = int(line.rstrip('\n'))
            first = False
        else:
            parsed = line.rstrip('\n').split ()
            array.append(Point(int(parsed[0]), int(parsed[1])))
    if expected_length != len(array):
        raise NameError("error on read")
    return  array

It reads list of points from file with format

<number of points>
x1 y1
x2 y2
share|improve this question
    
If you're dealing with points you might want to use a library like Shapely, which will give you a nice Point object. –  monkut Dec 14 '12 at 3:25
    
@monkut, thx I'll see. –  Stas Kurilin Dec 14 '12 at 9:46
add comment

3 Answers

up vote 4 down vote accepted

Another improvement is to use open as a context manager so that you don't have to remember to .close() the file object even if there are errors while reading.

def read():
    with open("FILE", "r") as f:
        array = []
        expected_length = int(f.next())
        for line in f:
            parsed = map(int, line.split())
            array.append(Point(*parsed))
        if expected_length != len(array):
            raise NameError('error on read')
        return array

See http://docs.python.org/2/library/stdtypes.html#file.close for more details.

share|improve this answer
    
Thanks. I've pick up several interesting features. –  Stas Kurilin Dec 14 '12 at 10:03
add comment

You don't need all that expected_length and first=True stuff. You can treat a file as an iterator, and it will return objects until it quits, so you can just use the .next() method and throw item away, or save it to a variable if you wish. In that sense, it's best to write two functions-- one to deal with the file, and another to deal with a single line as provided by the file object.

def lineparse(line):
    ''' Parses a single line. '''
    # your code goes here

def fileparse(filepath):
    f = open(filepath, "r")
    n = int(f.next()) # this also advances your iterator by one line
    while True: 
        yield lineparse(f.next())
    f.close()

data = list(fileparse(filepath))
share|improve this answer
    
expected_length is a check that the supposed length of the array equals the amount of data, you can't just ignore it. –  Stuart Dec 14 '12 at 3:54
    
Ah ignore me, I've just noticed you use n = to get the expected length. Still a bit confusing that you say you don't need expected_length though. –  Stuart Dec 14 '12 at 4:08
    
Thanks. I'll read about yield. –  Stas Kurilin Dec 14 '12 at 10:05
add comment

Not that big a change but you don't need to worry about stripping out \ns as the split and int functions will take care of that. Secondly, as already pointed out, you can grab the first line by just calling .next() then use a loop for the remaining lines.

def read():
    ins = open("FILE", "r")
    array = []
    expected_length = int(ins.next())
    for line in ins:
        parsed = line.split()
        array.append(Point(int(parsed[0]), int(parsed[1])))
    if expected_length != len(array):
        raise NameError('error on read')
    ins.close()
    return array
share|improve this answer
    
Thanks. Will know. –  Stas Kurilin Dec 14 '12 at 10:05
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.