patternpythonMinor
Inserting filename, reading data, inserting regex, and testing if each line matches
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
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 consideredFalse, so in your case you can just do something likeif not regex. If you need to test forNonespecifically, useis None.
- Don't do
return None, just an emptyreturnwill returnNoneautomatically.
- Don't do a blind
except:, you should always check for a specific exception.
- For the loop, use
enumerateto keep track of the current index.
- Use
with open(filename, 'r'):. This will automatically close the file when you leave thewithblock, 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 thanprint('"', regex, '"'). Or better yetregex = '"%s"' % regex if regex else 'empty', thenprint('Current regex is', regex).
show_menushouldn't be a separate function.
Context
StackExchange Code Review Q#94060, answer score: 8
Revisions (0)
No revisions yet.