patternpythonMinor
Q&A-based file editor
Viewed 0 times
fileeditorbased
Problem
As part of my autodidacticism into all things Python related, I have decided to write a number of functions that I can save in a library for future use. One such function that struck me as useful to have, would be a function that could be called each time the user wished to make an amendment to an existing file.
The result.
```
def amend_file_data(file_name) : # file_name to incl. extension.
#Opens existing file and amends the data according to user specifications.
#Local declarations:
file_contents = []
line_number = ""
amendments_complete = False
finalised_content = ""
further_amendments = ""
new_file_name = ""
check_intention = ""
file_contents = display_file_contents(file_name) # Displays contents of file and returns the contents as a list.
while not amendments_complete :
line_number = input("Please enter line number for amendment. \n"\
"To exit, please enter 'x': ")
while line_number.lower() != "x" :
line_number = int(line_number)
file_contents[line_number] = input("\nPlease enter replacement data: ") + "\n"
line_number = input("\nPlease enter line number for amendment. \n"\
"To exit, please enter 'x': ")
finalised_content = "".join(file_contents)
print("\nFile contents are now:\n" + finalised_content)
further_amendments = input("Do you wish to make any further amendments (Y/N): ")
if further_amendments.lower() == "n" :
amendments_complete = True
new_file_name = input("\nWould you like to keep the same file name? Y/N: ")
if new_file_name.lower() == "y" :
check_intention = input("\nPlease note, all previous data will be overwritten,"\
" do you want to proceed? Y/N: ")
if check_intention.lower() == "y".lower() :
with open(file_name,"w+") as new_file :
new_file.write(finalised_content)
else:
create_new_file(
The result.
```
def amend_file_data(file_name) : # file_name to incl. extension.
#Opens existing file and amends the data according to user specifications.
#Local declarations:
file_contents = []
line_number = ""
amendments_complete = False
finalised_content = ""
further_amendments = ""
new_file_name = ""
check_intention = ""
file_contents = display_file_contents(file_name) # Displays contents of file and returns the contents as a list.
while not amendments_complete :
line_number = input("Please enter line number for amendment. \n"\
"To exit, please enter 'x': ")
while line_number.lower() != "x" :
line_number = int(line_number)
file_contents[line_number] = input("\nPlease enter replacement data: ") + "\n"
line_number = input("\nPlease enter line number for amendment. \n"\
"To exit, please enter 'x': ")
finalised_content = "".join(file_contents)
print("\nFile contents are now:\n" + finalised_content)
further_amendments = input("Do you wish to make any further amendments (Y/N): ")
if further_amendments.lower() == "n" :
amendments_complete = True
new_file_name = input("\nWould you like to keep the same file name? Y/N: ")
if new_file_name.lower() == "y" :
check_intention = input("\nPlease note, all previous data will be overwritten,"\
" do you want to proceed? Y/N: ")
if check_intention.lower() == "y".lower() :
with open(file_name,"w+") as new_file :
new_file.write(finalised_content)
else:
create_new_file(
Solution
Docstring
The purpose and arguments for your functions should be documented using docstring.
Local declaration
You have habits of languages requiring variable declarations but there is not such thing in Python (even in languages requiring it, I consider it is better to declare variables as late as possible, in the smallest possible scope).
Style
Python has a style guide called PEP 8. You have various points that need to be changed to be compliant, mostly related to spacing. On the other hand, your are properly following the naming convention.
First draft
Once the comments above taken into account, the code looks like :
Duplicated logic
The following code does not respect the Don't Repeat Yourself principle:
I think it would be more clear to have :
User experience
From a user point of view, it would probably make sense to stop amendments when
Separation of concerns
It is a bit weird to have the
Performing simple substitutions, we have :
```
if new_file_name.lower() == "y":
check_intention = input("\nPlease note, all previous data will be overwritten,"
" do you want to proceed? Y/N: ")
if check_intention.lower() == "y".lower():
create_new_file(file_name, finalised_content)
else:
file_name = input("\nPlease enter a new file name (incl. .ext): ")
create_new_file(file_name, finalised_content)
else:
file_name = input("\nPlease enter a new file name (incl. .ext): ")
The purpose and arguments for your functions should be documented using docstring.
Local declaration
You have habits of languages requiring variable declarations but there is not such thing in Python (even in languages requiring it, I consider it is better to declare variables as late as possible, in the smallest possible scope).
Style
Python has a style guide called PEP 8. You have various points that need to be changed to be compliant, mostly related to spacing. On the other hand, your are properly following the naming convention.
First draft
Once the comments above taken into account, the code looks like :
def amend_file_data(file_name):
"""Opens existing file and amends the data according to user specifications.
file_name to incl. extension."""
finalised_content = ""
file_contents = display_file_contents(file_name) # Displays contents of file and returns the contents as a list.
while True:
line_number = input("Please enter line number for amendment. \n"
"To exit, please enter 'x': ")
while line_number.lower() != "x":
line_number = int(line_number)
file_contents[line_number] = input("\nPlease enter replacement data: ") + "\n"
line_number = input("\nPlease enter line number for amendment. \n"
"To exit, please enter 'x': ")
finalised_content = "".join(file_contents)
print("\nFile contents are now:\n" + finalised_content)
further_amendments = input("Do you wish to make any further amendments (Y/N): ")
if further_amendments.lower() == "n":
break
new_file_name = input("\nWould you like to keep the same file name? Y/N: ")
if new_file_name.lower() == "y":
check_intention = input("\nPlease note, all previous data will be overwritten,"
" do you want to proceed? Y/N: ")
if check_intention.lower() == "y".lower():
with open(file_name, "w+") as new_file:
new_file.write(finalised_content)
else:
create_new_file(finalised_content)
else:
create_new_file(finalised_content)
return print("\nWriting to file complete. ")
def create_new_file(finalised_content):
"""Creates a new file, accepting the file contents as an argument.
#Function accepts argument as string."""
new_file_name = input("\nPlease enter a new file name (incl. .ext): ") # File name to include .ext
with open(new_file_name, "w+") as new_file:
new_file.write(finalised_content)
return print("New file created successfully.")
def display_file_contents(file_name):
"""Displays contents of file to user and returns contents of file as a list.
file_name to include .ext"""
file_contents = []
with open(file_name, "r+") as file_for_display:
file_contents = file_for_display.readlines()
display_contents = "".join(file_contents)
print("Current contents of file:\n" + display_contents)
return file_contents # Returning file contents adds flexibility to the function, and allows
# further interaction with other functions.Duplicated logic
The following code does not respect the Don't Repeat Yourself principle:
line_number = input("Please enter line number for amendment. \n"
"To exit, please enter 'x': ")
while line_number.lower() != "x":
line_number = int(line_number)
file_contents[line_number] = input("\nPlease enter replacement data: ") + "\n"
line_number = input("\nPlease enter line number for amendment. \n"
"To exit, please enter 'x': ")I think it would be more clear to have :
while True:
line_number = input("Please enter line number for amendment. \n"
"To exit, please enter 'x': ")
if line_number.lower() == "x":
break
line_number = int(line_number)
file_contents[line_number] = input("\nPlease enter replacement data: ")User experience
From a user point of view, it would probably make sense to stop amendments when
x ig given as a line_number. Having 2 questions asked to exit is a bit un-natural.Separation of concerns
It is a bit weird to have the
create_new_file function to ask the user for a file name. Also, we have the open("w+") and write logic in 2 places. A better idea would be to have the file name given to the create_new_file function.Performing simple substitutions, we have :
```
if new_file_name.lower() == "y":
check_intention = input("\nPlease note, all previous data will be overwritten,"
" do you want to proceed? Y/N: ")
if check_intention.lower() == "y".lower():
create_new_file(file_name, finalised_content)
else:
file_name = input("\nPlease enter a new file name (incl. .ext): ")
create_new_file(file_name, finalised_content)
else:
file_name = input("\nPlease enter a new file name (incl. .ext): ")
Code Snippets
def amend_file_data(file_name):
"""Opens existing file and amends the data according to user specifications.
file_name to incl. extension."""
finalised_content = ""
file_contents = display_file_contents(file_name) # Displays contents of file and returns the contents as a list.
while True:
line_number = input("Please enter line number for amendment. \n"
"To exit, please enter 'x': ")
while line_number.lower() != "x":
line_number = int(line_number)
file_contents[line_number] = input("\nPlease enter replacement data: ") + "\n"
line_number = input("\nPlease enter line number for amendment. \n"
"To exit, please enter 'x': ")
finalised_content = "".join(file_contents)
print("\nFile contents are now:\n" + finalised_content)
further_amendments = input("Do you wish to make any further amendments (Y/N): ")
if further_amendments.lower() == "n":
break
new_file_name = input("\nWould you like to keep the same file name? Y/N: ")
if new_file_name.lower() == "y":
check_intention = input("\nPlease note, all previous data will be overwritten,"
" do you want to proceed? Y/N: ")
if check_intention.lower() == "y".lower():
with open(file_name, "w+") as new_file:
new_file.write(finalised_content)
else:
create_new_file(finalised_content)
else:
create_new_file(finalised_content)
return print("\nWriting to file complete. ")
def create_new_file(finalised_content):
"""Creates a new file, accepting the file contents as an argument.
#Function accepts argument as string."""
new_file_name = input("\nPlease enter a new file name (incl. .ext): ") # File name to include .ext
with open(new_file_name, "w+") as new_file:
new_file.write(finalised_content)
return print("New file created successfully.")
def display_file_contents(file_name):
"""Displays contents of file to user and returns contents of file as a list.
file_name to include .ext"""
file_contents = []
with open(file_name, "r+") as file_for_display:
file_contents = file_for_display.readlines()
display_contents = "".join(file_contents)
print("Current contents of file:\n" + display_contents)
return file_contents # Returning file contents adds flexibility to the function, and allows
# further interaction with other functions.line_number = input("Please enter line number for amendment. \n"
"To exit, please enter 'x': ")
while line_number.lower() != "x":
line_number = int(line_number)
file_contents[line_number] = input("\nPlease enter replacement data: ") + "\n"
line_number = input("\nPlease enter line number for amendment. \n"
"To exit, please enter 'x': ")while True:
line_number = input("Please enter line number for amendment. \n"
"To exit, please enter 'x': ")
if line_number.lower() == "x":
break
line_number = int(line_number)
file_contents[line_number] = input("\nPlease enter replacement data: ")if new_file_name.lower() == "y":
check_intention = input("\nPlease note, all previous data will be overwritten,"
" do you want to proceed? Y/N: ")
if check_intention.lower() == "y".lower():
create_new_file(file_name, finalised_content)
else:
file_name = input("\nPlease enter a new file name (incl. .ext): ")
create_new_file(file_name, finalised_content)
else:
file_name = input("\nPlease enter a new file name (incl. .ext): ")
create_new_file(file_name, finalised_content)
return print("\nWriting to file complete. ")
def create_new_file(file_name, content):
"""Creates a new file, accepting the file contents as an argument.
#Function accepts argument as string."""
with open(file_name, "w+") as new_file:
new_file.write(content)
return print("New file created successfully.")keep_file_name = False
new_file_input = input("\nWould you like to keep the same file name? Y/N: ")
if new_file_input.lower() == "y":
check_intention = input("\nPlease note, all previous data will be overwritten,"
" do you want to proceed? Y/N: ")
keep_file_name = check_intention.lower() == "y".lower()
new_file_name = file_name if keep_file_name else input("\nPlease enter a new file name (incl. .ext): ")
create_new_file(file_name, finalised_content)Context
StackExchange Code Review Q#118998, answer score: 7
Revisions (0)
No revisions yet.