I was making this file editing class (writing to a file, reading a file etc.) in Python and was just looking for any feedback.
Note: I've been thinking about removing the file_name
property and requiring file name on each method call.
import os
class EditFile:
"""
format: <object_name> = EditFile(<file_name>)
"""
# constructor
def __init__(self, file_name):
self.file_name = file_name # can be changed at any time
# string version
def __repr__(self):
return "<object_name> = EditFile(<file_name>)"
# methods
def write(self, text): # writes text in a file
try:
f = open(self.file_name, "w") # opens file & selects WRITE mode also creates a new file if it doesnt exist
f.write(str(text))
finally:
f.close()
def append(self, text): # appends text to a file
try:
f = open(self.file_name, "a") # opens file & selects APPEND mode
f.write(text)
finally:
f.close()
@property
def read(self): # reads the file
try:
f = open(self.file_name, "r") # opens file & selects READ mode
content = f.read() # saves content of the file into content variable
finally:
f.close()
return content # returns content of the file
def read_bytes(self, number_of_bytes = 0): # reads certain amount of bytes
try:
int(number_of_bytes)
if number_of_bytes == 0: # checks if its default
f = open(self.file_name, "rb")
content = f.read()
return content
elif number_of_bytes >= 1: # checks if number of bytes is a valid number
f = open(self.file_name, "rb")
content = f.read(number_of_bytes)
return content
else:
return "error" # returns "error" if input is invalid
except ValueError:
return "value error"
finally:
f.close()
def write_bytes(self, text): # writes in bytes
try:
f = open(self.file_name, "wb") # selects WRITE BYTES mode
f.write(str(text))
finally:
f.close()
@property
def clear(self): # clears a file
try:
f = open(self.file_name, "w") # opens file and selects WRITE mode
f.write("") # sets file content to none
finally:
f.close()
@property
def delete(self): # deletes current file
try:
os.remove(self.file_name) # tries to delete file
except OSError:
return "error" # returns "error" if an error occurs (eg file doesnt exist)
# example program using most of the methods
file = EditFile("editme.txt")
file.write(str(input("Text:\n")))
print(file.read)
file.clear
file.delete
# editing file: editme2.txt
file.file_name = "editme2.txt"
file.write(str(input("Text:\n")))
print(file.read_bytes(5))
file.delete
1 Answer 1
"<object_name> = EditFile(<file_name>)"
is not a particularly useful __repr__
. Note the guidance in the data model documentation:
If at all possible, this should look like a valid Python expression that could be used to recreate an object with the same value (given an appropriate environment). If this is not possible, a string of the form
<...some useful description...>
should be returned.
In this case, that could be:
"EditFile({!r})".format(self.file_name)
For all of your file access, you should use the context manager provided by open
, e.g.:
@property
def read(self): # reads the file
with open(self.file_name) as f:
return f.read()
This will handle closing the file for you after the method ends.
It seems weird that read
is a property but read_bytes
is a method - as the latter requires an argument, and both names describe actions, perhaps they should both be methods? If you still wanted the property then a name describing a thing, like content
, would be better for it.
I would generally expect attribute access not to change state, so it seems even weirder for clear
and delete
, again actions rather than things, to be properties. Just make them methods. Also the fact that they return either None
(if successful) or a string (if there was an error) is a really strange interface for a consumer of the class to deal with.
Your error handling in general is a bit odd. Why take a useful exception and squash it down to a meaningless string? You're giving the users of your class less information. If you can't add to them or usefully handle them, let the errors continue through your code to the caller. And rather than:
return "error" # returns "error" if input is invalid
raise an error with a useful message yourself, in this case e.g.:
raise ValueError("cannot read a negative number of bytes")
This actually tells the user something useful.
Also note that:
int(number_of_bytes)
although it checks that number_of_bytes
could be made an integer, doesn't actually change what's assigned to the name; if it was '1'
, for example, weird things will happen next. Instead, use the result:
number_of_bytes = int(number_of_bytes)
Rather than using simple comments to describe your methods, use docstrings:
def write_bytes(self, text):
"""Writes in bytes."""
...
Unlike comments these can be used by other tools, e.g. the built-in help
function, to make that information available in other forms.
self.file_name = file_name # can be changed at any time
Here is a good use for a @property
; you could use it to make the filename read-only by convention:
self._file_name = file_name
The leading underscore indicates that this should be considered private, then you can write a getter:
@property
def file_name(self):
return self.file_name
Your other idea, requiring the filename every time a method is called, would turn this into a class without any instance state, in which case it probably shouldn't be a class at all.
Explore related questions
See similar questions with these tags.