Disclaimer
I like your code, I like the idea of the notebook management application and I think your implementation is good. I just wanted to preface this because my post is very long and makes a lot of suggestions on what you could/should do different and quite a few errors I discovered and I don't want you to feel bad about this.
Useability and Errorhandling
Application crashes if storage file does not exist yet
I have tried out your code to get a feeling of how it works. If the program is started in a directory where no file named Note book.csv exists it will crash. This can be avoided pretty easily by adding a try-except block in the Delete_blank_line method:
output = ""
try:
with open("Note book.csv") as t:
for line in t:
if not line.isspace():
output += line
except:
pass
Application crashes when trying to list all notes but there are no notes
If there are no notes in the notebook, showing all notes fails with an exception. The reason for this is the del reader[0] line in the following code block (taken from the Show_all_the_name_or_note_of_the_notes method).
with open("Note book.csv", "r") as readfile:
reader = readfile.readlines()
del reader[0]
if not reader:
[...]
I do not understand the reason behind the del reader[0] statement which will just delete the first note and will fail if there are no lines. Removint it fixes the issue.
Application exits without message on invalid option
If the user enters an invalid option (anything other than the numbers 1-8), the application exits without any message. It would be nice to give feedback to the user to tell them they did something wrong and possibly give them the possibility to retry.
Application crashes after a certain amount of actions
I have to admit this is not something I found out while trying out your program but while analyzing the code. I have not been able to manually trigger the crash but was able to trigger it using yes 1 | python notebook.py. The application fails with a RecursionError.
The reason for this is that each function that handles an option will - after it has done it's job - call the Start_game method again. This can easily be avoided by instead returning to the Start_game method and inserting a while loop there (note you can then replace the exit() by break because the reason you needed to exit() in the first place was that your program was at a recursion of unknown depth):
def Start_Game(self):
while True:
n.Delete_blank_line()
n.print_option()
qus = input("Enter your choice here: ")
if qus == "1":
n.Show_all_the_name_or_note_of_the_notes(0)
[...]
if qus == "8":
print("Ok good bey")
break
Formatting
The formatting of your code looks really good, good job!. The things I noticed is that some of your variables and most of your methods use title snake case instead of lower snake case, e.g. The_Note should be the_note and your class name contains underscores instead of using PascalCase, this should be named NoteBokeFile instead of Note_Boke_File.
Coding Style
Using an global instance variable instead of self
Inside your class you use the global instance variable n to access methods, for example in Start_game:
def Start_Game(self):
n.Delete_blank_line()
n.print_option()
[...]
This is not a problem in the specific way your code is set up. However the functioning of the whole Note_Boke_File depends on how the instance is called. This can be fixed by replacing the name n by self everywhere except on the last two names:
def Start_Game(self):
self.Delete_blank_line()
self.print_option()
[...]
Reusing Variable Names
In the function Show_all_the_name_or_note_of_the_notes the reader variable is first used to store the lines read from file and then the variable name is reused to store a csv reader instance:
[...]
with open("Note book.csv", "r") as readfile:
reader = readfile.readlines()
del reader[0]
if not reader:
input(f"You don't have any Note yet\nPress enter to go back: "), n.Start_Game()
reader = csv.reader(reader)
for row in reader:
[...]
This is often a bad idea because it means having two different things, with two different datatypes beeing stored using the same name. This very easily overlooked when reading code and I have personally wasted numerous hours debugging programs where I inadvertedly did this.
In this case the result of readfile.readlines() should not be called reader but instead lines or lines_read. This avoids reusing the variable name.
Code Duplication
Many of the methods of the Note_Boke_File class read and write the storage file. Reading and writing the file for every action may not be strictly necessary but it has certain benefits, for example if the file was viewed or edited by another process while the notebook application is running. However, inserting the code for reading and writing the file to each method is not ideal. One of the reasons is that it is harder to maintain the code, because each change has to be made in every place. For example if you wanted to change the filename or writing the file in another format. There is already one mistake in your code in the Delete_blank_line method where the output file is not closed after writing (because there is no with statement and no t.close() call):
t = open("Note book.csv", "w")
t.write(output)
The solution for this would be to create two new methods read_file and write_file. This also allows you to get rid of the Delete_blank_line method by simply skipping empty lines in the read_file method.
Another repeated pattern is searching for a note by its name. This functionality is implemented in Add_note, Show_specific_note and Delete_note. All of those methods contain something along these lines:
note_name = input("Enter the name of the note you want to see: ")
control_when_to_exit_the_loop = 0
with open("Note book.csv", "r") as readfile:
reader = readfile.readlines()
reader = csv.reader(reader)
for row in reader:
if note_name in row:
print(f"Your note is:\n{row[1][1:]}"), input("Press enter to go back: ")
control_when_to_exit_the_loop += 1
if control_when_to_exit_the_loop == 0:
input("You don't have a Note with this name!\nPress enter to go back: "), n.Start_Game()
else:
[...]
This can first of all be refactored out of the individual methods. Instead you could create a method called find_note_by_name which returns the index of the note with the given name.
Integer Blindness
The method Show_all_the_name_or_note_of_the_notes takes an argument of type int to specify wheater it should print the whole note or only it's name. The issue with this is that it is not very readable. In the function itself the meaning of 1 and 0 can be infered from the context but at the call site it is not clear what 1 and 0 mean. See this blog post for more explaination about the blindness problem.
This could be improved by either using a name only boolean argument:
def Show_all_the_name_or_note_of_the_notes(self, *, show_only_name):
[...]
The * specifies that all of the arguments to the right may only be specified using the argument name. This forces the caller to use the method like this:
Show_all_the_name_or_note_of_the_notes(show_only_name=True)
However, a shortcomming of this is that it is not very open for future display options. The most pythonic way is to use a string argument as in the open(..., "w") method, where the "w" specifies the file should be opened for writing.
Since Python 3.8 there is also the Literal type in the typing module to specify which options are allowed. This provides documentation and tooling support for this usecase of strings.
from typing import Literal
[...]
def show_note(self, attributes: Literal["name", "content"]):
[...]
This could be extended to support displaying multiple attributes of a note by accepting a list of display options.
Long chain of if statements
In the Start_game method you use a number of if statements to decide what your program should do based on the option the user entered.
Because the options are mutually exclusive it is beneficial to use if-
elif-else here. This has two main advantages:
When reading the code no combinations of if statements have to be considered because in an if-elif-else statement only one of the blocks can ever be executed. On the other hand with consecutive if statements any number of cases could be executed depending on the content of the first executed block:
qus = '1'
if qus == '1':
do_something()
qus = '2':
if qus == '2':
qus = '3'
[...]
In this example, because the value of qus changes during excecution, multiple cases are executed. This can be avoided by using if-elif instead:
qus = '1'
if qus == '1':
do_something()
qus = '2'
elif qus == '2':
qus = '3'
[...]
In this case I know as a reader of the code that if qus has the value '1' the first code block and none of the others will be executed.
The second advantage of the elif is that we can add an else for the case that none of the options matched.
while True:
[...]
if qus == "1":
show_notes("name")
[...]
elif qus == "8":
print("Ok good bey")
break
else:
print(f"'{qus}' is not a valid option, please try again")
This will handle any invalid inputs and just let the user enter another option.
An even better option is available since Python 3.10: the match-case statement. With this you can avoid retyping qus on every option and even get rid of the variable altogether:
match input("Enter your choice here: "):
case "1":
show_notes("name")
case "2":
show_specific_Note()
case "3":
show_notes("content")
case "4":
delete_Note()
case "5":
add_note()
case "6":
number_of_note_you_have()
case "7":
restart_all()
case "8":
print("Ok good bey")
break
case user_input:
print(f"'{user_input}' is not a valid option, please try again")
Length of a list
In the method number_of_notes_you_have the amount of notes is calculated by iterating through the list and counting the elements:
note_number = 0
[...]
for row in reader:
note_number += 1
input(f"You have {note_number} Note\nPress enter to go back: ")
There is a more efficient way to do this (both in execution performance and in simplicity of the code). All the notes are in the list reader. The length of this list can be checked by using the builtin len function:
[...]
input(f"You have {len(reader)} Note\nPress enter to go back: ")
Indices in For Loops
Sometimes when writing code there is a need to iterate over the values of a list but also have the indices, as is the case in the Show_all_the_name_or_note_of_the_notes method:
note_number = 1
note_name = ""
[...]
reader = csv.reader(reader)
for row in reader:
note_name += f"{note_number}. {row[num]}\n"
note_number += 1
[...]
In this case you should avoid introducing a separate counting variable because this is error prone and harder to read (at least for experienced Python programmers). Instead the builtin enumerate method should be used:
note_name = ""
[...]
reader = csv.reader(reader)
for note_number, row in enumerate(reader, start=1):
note_name += f"{note_number}. {row[num]}\n"
note_number += 1
[...]
start=1 specifies that the first element gets the number 1 (default is 0).
if __name__ == "__main__":
In general it is a good idea to add if __name__ == "__main__": to any python file intended for execution (i.e. not a library module that only contains function and class definitions). This allows to use functions and classes to be imported to other python code without the part below if __name__ == "__main__": to be executed on import. See this answer on stackoverflow for more details.
Object Orientation
Your code consists of a single class of which only one instance is ever created. Combined with the complete lack of fields in your class there is actually no reason to have the class at all. There is nothing wrong with not having classes in Python code. As a side note, if you do have a class but your __init__ method is empty you can just get rid of it.
However, there is a very good reason to keep the class structure. If you want to make the filename of the notebook more flexible, you can pass the filename to the __init__ function, store it in a field and use the field everywhere in the class:
class Note_Boke_File:
def __init__(self, filename):
self.filename = filename
[...]
def Delete_blank_line(self):
output = ""
with open(self.filename) as t:
[...]
[...]
n = Note_Boke_File("Note book.csv")
n.Start_game()
I would also add an additional class for the notes. This helps to bring more structure into your code. At the moment you represent a note by the list of strings that you get from the csv.reader. To access the content of a note you use the index 1. If you were using a class for the nodes instead, you could access the content writing note.content:
class Note:
def __init__(self, title, content):
self.title = title
self.content = content
note = Note('mytitle', 'This is the content of my note')
print(note.content)
Applying the Suggestions
The following refactored code should serve as an example on how applying all the suggestions to your code could look:
import csv
from typing import Literal
MISSING_INDEX = -1
class Note:
def __init__(self, name, content):
self.name = name
self.content = content
class NoteBook:
def __init__(self, filename, backup_filename=None):
self.filename = filename
# if the backup filename is not specified, generate a new
# backup filename by adding ".bkup" to the filename
if backup_filename is None:
self.backup_filename = filename + ".bkup"
else:
self.backup_filename = backup_filename
def start_game(self):
self.create_backup()
while True:
print(
"\n1. Show all the name of the Notes\n"
"2. Show specific Note\n"
"3. Show all your note\n"
"4. Delete Note\n"
"5. Add Note\n"
"6. Show how much Note i take until now\n"
"7. Restart all\n"
"8. Exit"
)
match input("Enter your choice here: "):
case "1":
self.show_notes("name")
case "2":
self.show_specific_note()
case "3":
self.show_notes("content")
case "4":
self.delete_note()
case "5":
self.add_note()
case "6":
self.number_of_note_you_have()
case "7":
self.restart_all()
case "8":
print("Ok good bey")
break
case option:
print(f"\n'{option}' is not a valid option")
def add_note(self):
name_note = input("Enter the name of the Note: ")
notes = self.read_file()
if self.find_note_by_name(name_note, notes) != MISSING_INDEX:
input(
"You already have this name try another one!\nPress enter to go back: "
)
return
else:
the_note = input("Enter your Note here: ")
notes.append(Note(name_note, the_note))
self.write_file(notes)
input("Your note added successfully!\nPress enter to go back: ")
def show_notes(self, attribute: Literal["name", "content"]):
match attribute:
case "name":
name = "Note name"
case "content":
name = "Note"
case _:
raise ValueError(f"Unknown attribute: {attribute}")
notes = self.read_file()
if len(notes) == 0:
input(f"You don't have any Note yet\nPress enter to go back: ")
return
note_name = ""
for note_number, note in enumerate(notes, start=1):
note_name += f"{note_number}. {getattr(note, attribute)}\n"
input(f"All your {name} are:\n{note_name}\nPress enter to go back: ")
def show_specific_note(self):
note_name = input("Enter the name of the note you want to see: ")
notes = self.read_file()
note_index = self.find_note_by_name(note_name, notes)
if note_index == MISSING_INDEX:
input("You don't have a Note with this name!\nPress enter to go back: ")
else:
print(notes[note_index].content)
def restart_all(self):
try:
with open(self.backup_filename, "r") as readfile:
lines_read = readfile.read()
with open(self.filename, "w") as writefile:
writefile.write(lines_read)
print("Everything restarted!")
except:
print("Could not restore from backup")
def create_backup(self):
try:
with open(self.filename, "r") as readfile:
lines_read = readfile.read()
with open(self.backup_filename, "w") as writefile:
writefile.write(lines_read)
except:
print("Warning: Could not create backup")
def number_of_note_you_have(self):
notes = self.read_file()
if len(notes) == 0:
input(f"You don't have any Note yet\nPress enter to go back: ")
else:
input(f"You have {len(notes)} Note\nPress enter to go back: ")
def delete_note(self):
notes = self.read_file()
name_note = input("Enter the name of the Note: ")
note_index = self.find_note_by_name(name_note, notes)
if note_index == MISSING_INDEX:
input("You don't have a Note with this name!\nPress enter to go back: ")
else:
notes.remove(notes[note_index])
self.write_file(notes)
print("The Product remove successfully!\nPress enter to go back: ")
def find_note_by_name(self, name: str, notes: list[Note]) -> int:
for index, note in enumerate(notes):
if note.name == name:
return index
else:
return MISSING_INDEX
def read_file(self) -> list[Note]:
try:
with open(self.filename) as t:
reader = csv.reader(t)
return [Note(row[0], row[1]) for row in reader if row != []]
except FileNotFoundError:
return []
def write_file(self, notes: list[Note]):
with open(self.filename, "w") as writeFile:
writer = csv.writer(writeFile)
writer.writerows([[note.name, note.content] for note in notes])
if __name__ == "__main__":
notebook = NoteBook(filename="Note book.csv")
notebook.start_game()
I was not quite sure what the intent behind the restart_all method was. The above implementation uses a backup file created at program start to restore the notes to this state when restart_all is called.
