2
\$\begingroup\$

I currently have this code to read lines from a file

def getLinesFromFile(filename):
    content = []

    # with open(filename, "r+") as file:
    #   for line in file:
    #       line = line.rstrip()
    #       if line:
    #           content.append(line)

    file = None
    try:
        file = open(filename, "r+")
        for line in file:
            line = line.rstrip()
            if line:
                content.append(line)
    except IOError:
        print "[ERR] File does not exist"
    finally:
        if file:
            file.close()

    return content

Please take note of the commented out code using with. I really wanted to use with but I can't find any good resource on how to catch exceptions with with. So I did it using try except.

Question: Is there no better way to do this? Or any comments?

P.S. I come from a Java background and this is my first time writing in Python so I am more inclined to try-except as of the moment.

\$\endgroup\$
7
\$\begingroup\$

You can use try except IOError on with the same way you did for open.

def getLinesFromFile(filename):
    content = []

    try:
        with open(filename, "r+") as file:
            for line in file:
                line = line.rstrip()
                if line:
                    content.append(line)
    except IOError:
        print "[ERR] File does not exist"

    return content

Using with is a good idea, but I'm not sure if you understood what it does. with automatically ensures that a file is always closed safely. So even if the script encounters an error then you'll close it. This means that you only need to catch an error if the file doesn't exist, which is why we use except IOError like this.

There are other notes too. In Python it's recommended that you use snake_case, not camelCase like in java. So you should call your function get_lines_from_file.

There's no need to use "r+", that's for reading and writing a file, but you only read it. Instead you could use 'r' but since that's the default you can just leave out the mode argument and do open(filename).

Don't use the name file, as that's shadowing Python's built in file constucting function. Python allows you to use a lot of the built in method's names in case you want to overwrite them, but it's best not to unless it's necessary. Using f is fine in such a small function.

You can also build your list easier with something called a list comprehension. List comprehensions are like for loops collapsed into a single expression that results in a list. In your case, you first need to get every line from the list and rstrip it, so that'd look like this:

content = [line.rstrip() for line in f]

However you also want to ignore empty lines, so you'll want to add an if condition at the end:

content = [line.rstrip() for line in f if line.rstrip()]

Now, you don't even need to actually store this as content. You can return it directly instead. Though, doing this means that you should manually return an empty list when the IOError is raised, otherwise Python will default to returning None. (It might be better to return None in the case of an error, but that depends on how you're using this)

Lastly, add a docstring to explain what the function does. They're a similar idea to javadocs but less structured and formal. Here's how I'd write the function:

def get_lines_from_file(filename):
    """Return a list of rstripped lines from filename

    Returns an empty list if the file doesn't exist."""

    try:
        with open(filename) as f:
            return [line.rstrip() for line in f if line.rstrip()]
    except IOError:
        print "[ERR] File does not exist"
        return []
\$\endgroup\$
  • \$\begingroup\$ Two questions for you: Doesn't you change behaviour when you replace rstrip with strip? How bad is it to do the strip operation twice for each line? \$\endgroup\$ – holroy Nov 2 '15 at 14:56
  • \$\begingroup\$ Ah, the r was a typo, good catch! And it's not a big deal. rstrip can tun a million times in a fraction of a second (tested with timeit) and the list comprehension more than makes up for that over a for list calling append. \$\endgroup\$ – SuperBiasedMan Nov 2 '15 at 15:00
  • 1
    \$\begingroup\$ Good lord you are a lifesaver! I really learned a lot just from your post! I think I am twice as good of a Python programmer now just because I read your post! I wish I could upvote this more than once. Thanks! \$\endgroup\$ – Incognito Nov 2 '15 at 15:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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