HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

MS-DOS style OS

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
styledosstackoverflow

Problem

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

Solution

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 comma

Code Snippets

COMMANDS = {
    'cd' : cd,
    'cr' : cr,
    # 'leave' : quit(), # Is specially handled, but listed for reference
    'fopen' : fopen,
    ...
}
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()

Context

StackExchange Code Review Q#120582, answer score: 7

Revisions (0)

No revisions yet.