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

Inserting filename, reading data, inserting regex, and testing if each line matches

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

Problem

This program first displays a menu which shows some options that the user can select. Each option will the respective functions which do some things.

The user can insert a filename, read data from it, enter a regex pattern, test if each line of the file matches the given regex pattern.

Please provide feedback on how I can improve my program.

```
import re

filename = None
readData = None
regex = None

def test_data():
'''Checks if each line of data matches the regex'''

global readData
global regex

if regex == None:
print('Empty regex')
print('')
return None

try:
pattern = re.compile(regex)
except:
print('Invalid regex')
print('')
return None
i = 1
if readData == None:
print('Read data is empty')
print('')
return None

for line in readData:
if pattern.match(line.strip()):
print(' Line ', i, ':"', line,'"', ' matches current regex', sep='')
else:
print(' Line ', i, ':"', line,'"', ' does not match current regex', sep='')
i += 1
print('')

def change_regex():
'''Sets the regex variable'''

global regex
print('Current regex is ', end='')
if regex != None:
print('"', regex, '"', sep='')
else:
print('empty')

regex = input('Enter new regex: ')
print('')

def read_file():
'''Reads data from file if possible'''

global filename
global readData
try:
file = open(filename, "r")
readData = file.readlines()
print('Data read successfully!')
file.close()
except TypeError:
print('filename is empty!')
except FileNotFoundError:
print('File not found!')

print('')

def change_filename():
'''Sets the filename variable'''

global filename
print('Current filename is ', end='')
if filename != None:
print('"', filename, '"', sep='')
else:
print('empty')

file

Solution

Some thoughts:

  • Don't use globals, pass variables as function arguments.



  • Follow pep8



  • Don't use == None. False, 0, 0.0, None, and empty sequences like lists or strings are all considered False, so in your case you can just do something like if not regex. If you need to test for None specifically, use is None.



  • Don't do return None, just an empty return will return None automatically.



  • Don't do a blind except:, you should always check for a specific exception.



  • For the loop, use enumerate to keep track of the current index.



  • Use with open(filename, 'r'):. This will automatically close the file when you leave the with block, even if there is an exception, so you don't need to manually close.



  • In read_file, don't catch the exception at all. The default exceptions give the user more information and more useful information than your exceptions, so just let them be handled normally.



  • It is clearer to do something like print('"%s"' % regex) rather than print('"', regex, '"'). Or better yet regex = '"%s"' % regex if regex else 'empty', then print('Current regex is', regex).



  • show_menu shouldn't be a separate function.

Context

StackExchange Code Review Q#94060, answer score: 8

Revisions (0)

No revisions yet.