4
\$\begingroup\$

This is a very basic text-based operating system I've been working on. I started working with python a few weeks ago and this project only a few days ago, so there are probably better ways I can do things, but I was wondering what people think of it.

NOTE: I know that the edit function doesn't work.

import os

def user():
    makeUser()

def edit():
    file = input("Which file do you want to edit? ")
    with open(file, 'w+') as f:
        filetext = ("")
        for line in f:
            filetext += line
        f.write(filetext)

def fopen():
    file = input("Open: ")
    print("")
    with open(file, 'r') as f:
        for line in f:
            print(line)
            print("")
    editredirect()

def editredirect():
    print("You can edit this file with the 'edit' command.")
    dcmdLvl2()  

def cr():
    directoryName = input("Name of directory: ")
    os.mkdir(directoryName)
    if not os.path.exists(directoryName):
        os.mkdir(directoryName)
        dcmdLvl2()

def cd():
    changedDIR = input("Directory name: ")
    os.chdir(changedDIR)
    if not os.path.exists(changedDIR):
        print("Directory not found.")
        dcmdLvl2()

def rem():
    selectFile = input("Which file do you want to delete? ")
    os.remove(selectFile)   

def cmdLvl2():
    print("Use the 'leave' command to shut down the system. Use the 'type' command to start a text editor. You can also type 'clear' to clear the screen. Type 'help -a' for more options.")
    tcmdLvl2 = input("~$: ")
    if tcmdLvl2 == ("leave"):
        quit()
    if tcmdLvl2 == ("type"):
        typer()
    if tcmdLvl2 == ("clear"):
        os.system('cls')
    if tcmdLvl2 == (""):
        dcmdLvl2()
    if tcmdLvl2 == ("help -a"):
        print("You can use the command 'cr' to make a new directory. Use the 'cd' command to access this directory. Additionally, use 'list' to show all sub-folders in your current directory or use 'show' to see your current directory. You can also use 'rem' to remove a file.")
        dcmdLvl2()
    if tcmdLvl2 == ("cr"):
        cr()
    if tcmdLvl2 == ("cd"):
        cd()
    if tcmdLvl2 == ("list"):
        os.system('dir')
    if tcmdLvl2 == ("show"):
        print(os.getcwd())
    if tcmdLvl2 == ("rem"):
        rem()
    if tcmdLvl2 == ("open"):
        fopen()
    if tcmdLvl2 == ("edit"):
        edit()
    if tcmdLvl2 == ("help"):
        cmdLvl2()
    if tcmdLvl2 == ("user"):
        user()
    if tcmdLvl2 not in ("leave", "type", "clear", "", "help", "help -a", "cr", "cd", "list", "show", "rem", "open", "edit", "user"):
        print("Command not found.")
    dcmdLvl2()

def dcmdLvl2():
    tcmdLvl2 = input("~$: ")
    if tcmdLvl2 == ("leave"):
        quit()
    if tcmdLvl2 == ("type"):
        typer()
    if tcmdLvl2 == ("clear"):
        os.system('cls')
    if tcmdLvl2 == (""):
        dcmdLvl2()
    if tcmdLvl2 == ("help"):
        cmdLvl2()
    if tcmdLvl2 == ("cr"):
        cr()
    if tcmdLvl2 == ("cd"):
        cd()
    if tcmdLvl2 == ("list"):
        os.system('dir')
    if tcmdLvl2 == ("show"):
        print(os.getcwd())
    if tcmdLvl2 == ("rem"):
        rem()
    if tcmdLvl2 == ("open"):
        fopen()
    if tcmdLvl2 == ("edit"):
        edit()
    if tcmdLvl2 == ("user"):
        user()
    if tcmdLvl2 == ("help -a"):
        print("You can use the command 'cr' to make a new directory. Use the 'cd' command to access this directory. Additionally, use 'list' to show all sub-folders in your current directory or use 'show' to see your current directory. You can also use 'rem' to remove a file.")
        dcmdLvl2()
    if tcmdLvl2 not in ("leave", "type", "clear", "", "help", "help -a", "cr", "cd", "list", "show", "rem", "open", "edit", "user"):
        print("Command not found.")
    dcmdLvl2()

def typer():
    print("")
    print("Start typing to begin.")
    typerCMD = input("  ")
    saveAs = input("Save file as: ")
    if saveAs == (""):
        dcmdLvl2()
    with open(saveAs, 'w') as f:
        f.write(typerCMD)
    print("")
    print("You can access this file with the 'open' command.")
    print("")
    dcmdLvl2()

def CMDLine():
    print("Hello, and welcome to your new operating system. Type 'help' to get started.")
    cmd = input("~$: ")
    if cmd == ("leave"):
        quit()
    if cmd == ("type"):
        typer()
    if cmd == ("clear"):
        os.system('cls')
    if cmd == (""):
        dcmdLvl2()
    if cmd == ("help"):
        cmdLvl2()
    if cmd == ("cr"):
        cr()
    if cmd == ("cd"):
        cd()
    if cmd == ("list"):
        os.system('dir')
    if cmd == ("show"):
        print(os.getcwd())
    if cmd == ("rem"):
        rem()
    if cmd == ("help -a"):
        print("You can use the command 'cr' to make a new directory. Use the 'cd' command to access this directory. Additionally, use 'list' to show all sub-folders in your current directory or use 'show' to see your current directory. You can also use 'rem' to remove a file.")
    if cmd == ("open"):
        fopen()
    if cmd == ("edit"):
        edit()
    if cmd == ("user"):
        user()
    if cmd not in ("leave", "type", "clear", "", "help", "help -a", "cr", "cd", "list", "show", "rem", "open", "edit", "user"):
        print("Command not found.")
    dcmdLvl2()

def redirect():
    signIn()

def mUserRedirect():
    makeUser()

def PwordSignIn():
    rPword = input("Password: ")
    with open('passwords.txt', 'r') as f:
        for line in f:
            if rPword == (line):
                CMDLine()
            else:
                print("Incorrect password.")
                signIn()

def signIn():
    rUname = input("Username: ")
    with open('usernames.txt', 'r') as f:
        for line in f:
            if rUname == (line):
                PwordSignIn()
            else:
                print("Username not found.")
                mUserRedirect()

def makeUser():
    nUname = input("New username: ")
    nPword = input("Create a password for the user: ")

    with open('usernames.txt', 'w') as f:
        f.write(nUname)
    with open('passwords.txt', 'w') as f:
        f.write(nPword)
    signIn()

print("Create a new user? Warning: This will delete any other users. (Y/N): ")
nUser = input(" ")
if nUser == ("N"):
    signIn()
if nUser == ("n"):
    signIn()
if nUser == ("Y"):
    makeUser()
if nUser == ("y"):
    makeUser()
\$\endgroup\$

2 Answers 2

7
\$\begingroup\$

Here are some general comments to your code, where a lot of these are related to following the guidelines of PEP8:

  • Variable and function names are normally snake_case – In addition the names are normally spelled out, and not abbreviated. I.e. instead of cmd use command, instead of CMDLine use command_line, instead of makeUser use make_user
  • Simplify string comparison – Instead of cmd == ("leave"), you can do cmd == "leave". No need for the extra parenthesis
  • Try avoid long sequence of if statements – Usually when you get long sequences like in your CMDLine(), cmdLvl2() and dcmdLvl2(), you could most likely simplify it somehow. Will show this lower down
  • Avoid endless recursive loops – All of your commands end calling dcmdLvl2(), which in turn creates a endless loop. You would be much better of using a while loop trigger different commands, and just returning after the command triggering a new command run
  • Combine equal conditions if possible – Instead of calling signIn() from two different loops you could make the condition either into nUser == "N" or nUser == "n" or possibly you could use nUser.lower() == "n". Or even better lower case all commands in the input line, i.e. command = input("~$: ").lower().
  • Introduce and use a main() function – A common idiom seen in Python is the if __name__ == '__main__': followed by main() on the next line. In your case the last few lines would be part of this main() function.

    The advantage of this approach is that you allow for your code to be imported as a module later on from where you can call the different functions, whilst still allowing for the script to be called from the command line triggering your self written shell.

  • Using dict of methods to avoid the if sequences – In Python you can make dicts of the command with corresponding methods, which would greatly simplify your program. This can be done with a dict like the following:

    COMMANDS = {
        'cd' : cd,
        'cr' : cr,
        # 'leave' : quit(), # Is specially handled, but listed for reference
        'fopen' : fopen,
        ...
    }
    

    In this code excerpt I've used your original name, but I would suggest using longer better names.

Bugs or features

In addition to the stylistic points of previous sections here are some points to stuff which you haven't addressed or are currently ignoring.

  • fopen can list password – You've not implemented any security, so the fopen might easily list current password
  • Only one user allowed – If you create a new user, the previous is removed. So why bother having this user level?
  • Password is stored in plain text – Your password is stored as plaintext which isn't good, simply put.
  • None of your commands allows arguments – This triggers a need for all of your commands to have a second input in order to do anything really useful. If you allowed arguments, and used split() when checking for commands, you allow for only one input pr command instead of two for some of them
  • In signIn() you keep looping... – Theoretically if you returned from the PwordSignIn(), you would continue matching lines from usernames.txt. This could/should be rewritten
  • All files are parallell to your script – It would most likely be good to limit your command shell to operate on it's own set of files. I.e. make a folder to be used by the command shell, and not allow stepping out of this folder. Unless you actually want to operate on the existing files, that is.

A starter for rewrite

Here is some code to help you start a bigger rewrite implementing the one-time command input, and grouping the code somewhat differently:

def change_directory(new_directory):
    """Change directory to the named directory."""

    if os.path.exists(new_directory):
        os.chdir(new_directory)
    else:
        print("Directory not found.")


def create_directory(new_directory):
    """Create the named directory."""

    if not os.path.exists(new_directory):
        os.mkdir(new_directory)
    else:
        print("Directory, {}, already exists.".format(new_directory))


COMMANDS = {
  'cd' : change_directory,
  'cr' : create_directory,
  ...
}


def command_line():
    """Reads a command, and executes it if found."""

    args = input("~$: ").lower().split()

    command = args[0]

    if command == "leave":
        break

    elif command == "help" and args[1] in COMMANDS:
        print("Help for {}: {}".format(args[1], COMMANDS[args[1]].__doc__))

    elif command in COMMANDS:
        COMMANDS[command](*args[1:])

    else:
        print("Unknown command, please try again or use 'leave' to quit")



def main():

    print("Before starting the interactive command shell, please log in")
    if sign_in():
       command_line()

    print("Thank you for using the interactive command shell")


if __name__ == '__main__':
    main()

This utilises docstring's for presenting help, i.e. try help cd, and it uses the split() to split the command line into arguments. And it shows how this can be passed to accompanying functions.

Hopefully this (untested) code can help you to a new better version of your command, and at the same time learn you some new tips and tricks in Python programming.

\$\endgroup\$
1
  • \$\begingroup\$ Very helpful comments and suggestions. I did want to comment that to make PEP8 easier, you can do ( ) around things to help with line breaks. re: Simplify string comparison – Instead of cmd == ("leave"), you can do cmd == "leave". It is not necessary, but if you have a long string to assign you can break it across multiple line by encapsulating it in ( ) and then no need for the \ characters and then challenges with spacing and tabs. You can also look into flake and pep8 lint tools for your editors that will help with conventions like pep8, pep256, etc. \$\endgroup\$
    – cgseller
    Commented Feb 20, 2016 at 13:21
4
\$\begingroup\$

The fundamental problem with this code, which @holroy alluded to but didn't state explicitly, is that you are misusing function calls as if they are GOTOs. This is an inappropriate way to structure your program, but it's a common mistake for Python beginners. As a result,

  • The call stack is inappropriately deep after running for a while.
  • The program is "spaghetti code", since any portion can jump to any other place with no obvious structure.
  • Data doesn't flow in your program: none of your functions accepts any parameters, and none of them returns any result.
  • There is very little code reuse within your program if at all.
  • You can't have decent error handling.

The program needs to be completely rewritten to fix this problem. There will need to be a while loop that is responsible for printing the prompt, accepting the user input, parsing it, and dispatching the action. See the cited examples in my link for how programs like this should be reorganized.

\$\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.