HiveBrain v1.2.0
Get Started
← Back to all entries
debugpythonModerate

Read through a log file to find "Fail" lines

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
findfilelogreadfailthroughlines

Problem

Please bear with me, I'm only about a week into Python.

This code is something that I wanted to do once I completed the chapters dealing with opening and reading files, and RegEx. I always feel like I'm over complicating the code. Please take a look and let me know if this is sound and where it can be improved. There are a lot of concepts I haven't learned yet. I'm teaching myself out of a book.

This code is meant to open a log file (will eventually be retrieved real-time), go through it line by line using RegEx to find "Fail", add those lines reporting failures to a list and return the completed list as a string joined with new lines.

#!/usr/bin/python

import os
import re

myFile = '/home/nick/python/chap8ReadingWritingFiles/15Jan2016_000000.txt'

def RegEx(file):
    openFile = open(myFile, 'r')
    myResList = []
    myList = openFile.readlines()
    myReg = re.compile(r'Fail')
    for i in (myList):
            #print(i, end='')
            result = myReg.search(i)
            if result != None:
                    #print(i, end='')
                    myResList.append(i.rstrip())
    final = '\n'.join(myResList[:])
    return final

print()
print('Results:')
print(RegEx(myFile))
print()

Solution

The standard indentation for Python, specified by PEP 8, is four spaces. This is a pretty strong convention for Python, since indentation matters a lot.

You almost always want to call open() in the context of a with-block, so that the file will get closed automatically when exiting the block.

This task does not require you to read the entire file at once. I recommend working line by line so that you can handle large files better. (You also don't need to make a copy of myResList at the end using myResList[:].)

You don't need a regular expression to search for a fixed literal string. A substring search will do just fine. This also means that your function is misnamed. In general, you should name functions after the task that they accomplish, rather than how they accomplish the task.

I'd generalize your function to search for any text.

def search(filename, text):
    with open(filename) as f:
        for line in f:
            if text in line:
                print(line)

if __name__ == '__main__':
    search('/home/nick/python/chap8ReadingWritingFiles/15Jan2016_000000.txt', 'Fail')


If you're interested more in passing back the results, a better way would be to yield the results back to the caller.

def search(filename, text):
    with open(filename) as f:
        for line in f:
            if text in line:
                yield line

if __name__ == '__main__':
    for result in search('/home/nick/python/chap8ReadingWritingFiles/15Jan2016_000000.txt', 'Fail'):
        print(result)


The if __name__ == '__main__': boilerplate is optional, but considered standard practice.

Code Snippets

def search(filename, text):
    with open(filename) as f:
        for line in f:
            if text in line:
                print(line)

if __name__ == '__main__':
    search('/home/nick/python/chap8ReadingWritingFiles/15Jan2016_000000.txt', 'Fail')
def search(filename, text):
    with open(filename) as f:
        for line in f:
            if text in line:
                yield line

if __name__ == '__main__':
    for result in search('/home/nick/python/chap8ReadingWritingFiles/15Jan2016_000000.txt', 'Fail'):
        print(result)

Context

StackExchange Code Review Q#117206, answer score: 10

Revisions (0)

No revisions yet.