2
\$\begingroup\$

I want to make a command-line text editor in python. Is this good code?

from termcolor import * import os while True: files = os.listdir() bufname = input("File name: ") title = cprint("Ganesha Editor v1.0", "green") controls = cprint("Type $exit to exit text editor. It autosaves.", "blue") def write(): print("Compose Something Below:\n\n") text = input() if text == "$exit": exit() else: if bufname in files: os.remove(bufname) create = open(bufname, "w") create.close() autosave = open(bufname, "a") autosave.write(text) autosave.write("\n") autosave.close() else: autosave = open(bufname, "a") autosave.write(text) autosave.write("\n") autosave.close() while True: write() 

I prettified the code with 8 spaces for the indents.

Does anyone like this code? It's very minimal, and no one will like it, but, I'm trying to make a secondary text-editor.

\$\endgroup\$

    1 Answer 1

    3
    \$\begingroup\$

    The code's pretty straightforward, but it could still be simplified a bit:

    1. There's no reason to have multiple levels of while loop; you already run your write() function in an inner loop until it's time to exit, so the outer loop only ever gets executed once.

    2. The code to append to the file is duplicated.

    3. The file deletion behavior is very strange and seems like a bug -- if the file exists, you delete it, but you do that after every line, making it impossible to write a multi-line file (but only in the case where the file existed when you started the editor). Seems like you probably meant to only delete it once but forgot to update your files list?

    4. The write() function doesn't necessarily always write something; sometimes it exits (which is generally considered bad form since it makes reuse and testing difficult). It also has the extra responsibility of deleting the existing file, which is why that deletion is maybe happening more often than you meant it to. Separating the setup from the core editing loop would make this much easier to reason about.

    5. cprint doesn't return a value, so assigning it to title and controls (which you don't use anyway) is confusing.

    6. When an if block breaks the control flow (e.g. by exit() or return or break) it's not necessary to have everything else under an else. In some cases the code may be more clear with a redundant else (e.g. when the two branches are similar in length and/or when you want to make it more obvious that they're mutually exclusive), but when the bulk of your code is indented unnecessarily it tends to make it harder to follow rather than easier.

    7. It's generally better to use file handles as context managers (with open ...). It makes it impossible to forget to close the file, and it makes it very obvious what you're doing while the file is open.

    8. This is more of a note on the UX than the code, but I suggest (A) telling the user when you're deleting their file (and only doing it on startup rather than after every autosave) and (B) not having so many blank/repeated lines in the interface (it makes it harder to see what you've already entered).

    Below is the result of applying the above notes -- now there are two functions (init and run), with one in charge of everything that happens up front, and the other in charge of everything that happens on each subsequent input.

    import os from termcolor import cprint def init() -> str: """ Start the editor by prompting for a filename and printing usage information. If the file already exists, remove it. Returns the filename. """ filename = input("File name: ") if filename in os.listdir(): print("Deleting existing file to give you a blank slate.") os.remove(filename) cprint("Ganesha Editor v1.0", "green") cprint("Type $exit to exit text editor. It autosaves.", "blue") print("Compose Something Below:") return filename def run(filename: str) -> bool: """ Prompts for input and writes it to the file. Returns True as long as it's still running, returns False when it's time to exit. """ text = input() if text == "$exit": return False with open(filename, "a") as autosave: autosave.write(text) autosave.write("\n") return True if __name__ == "__main__": filename = init() while run(filename): pass 
    \$\endgroup\$

      Start asking to get answers

      Find the answer to your question by asking.

      Ask question

      Explore related questions

      See similar questions with these tags.