3
\$\begingroup\$

I am fairly new to Python and I'm trying to make a script that does three basic things: prints the contents of a file, overwrites it with your new lines, and prints the new file. I rewrote the script with functions and was looking for critiques. Is this a good use of functions? What would you do different?

print("Type the name of the file for overwriting.")
file = input(">")
x = open(file)
print("The contents are currently: \n", x.read())
print("Enter in the new lines")
l1 = input(">")
l2 = input(">")
l3 = input(">")
x.close()

y = open(file, 'w')

print("Entering new lines.")

y.write(l1 + '\n' + l2 + '\n' + l3)
y.close()

z = open(file).read()
print("The new contents are:")
print(z)

Script with functions:

def open_and_read(x):
    print(open(x).read())

def nl(x):
    z.write(x + '\n')

temp = ">"  
print("Type the name of the file for overwriting.")
file = input(temp)
print(f"The current content of {file} is:")
open_and_read(file)

z = open(file, 'w')

print("Enter in three new lines of text:")
l1 = input(temp)
l2 = input(temp)
l3 = input(temp)

nl(l1)
nl(l2)
nl(l3)

z.close()

print("The new contents are:")
open_and_read(file)
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

First of all, you should definitely switch to using with context manager which is gonna safely and automatically close the opened file handler.

Also, you need to improve your variable names, l1, l2, l3 or z are not descriptive.

And, you may use .writelines() to write a list of strings each of them on a separate line.

In the end, you might get something along these lines:

print("Type the name of the file for overwriting.")
file_name = input(">")

with open(file_name) as input_file:
    print("The contents are currently: \n", input_file.read())
    print("Enter in the new lines")

lines = [input(">") for _ in range(3)]

print("Entering new lines.")
with open(file_name, 'w') as output_file:
    output_file.writelines(lines)

with open(file_name) as input_file:
    print("The new contents are:")
    print(input_file.read())
\$\endgroup\$
3
\$\begingroup\$

Is this a good use of functions?

Not really, take this function for example:

def nl(x):
    z.write(x + '\n')

It is not descriptive at all

  1. Where does the variable z comes from?
  2. z is a global variable out of scope of the function. (which is BAD)
  3. What does nl stand for?

When writing good functions it's better to have longer descriptive names. Your other function open_and_read() has a way better name, although that could also use some improvement.

How to write good function?

def write_to_file(filename, content):
    """Writes content to a file."""
    with open(filename, 'w') as file:
        file.write_lines(content)

This would be a better function, because.

  1. Has descripte variable names
  2. A nice doctring to explain what the function does
  3. A function name, where it is clear on first sight what that function does
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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