10
\$\begingroup\$

I made a command line in Python. Before I go on and add more commands, is my program made well?

I don't like the huge amount of "if" statements in it. But I don't know a better way to do it. (JSON may be used later this is why I have it included)

Here's the code:

import time
import json
print("Welcome to pyCMD, user\n")
time.sleep(0.2)
input("Press enter to start\n")
time.sleep(0.2)

def SavetoDisk(data, file):
    with open(file, 'w') as outfile:
        json.dump(data, outfile)

def ReadfromDisk(file):
    with open(file) as json_file:
        data = json.load(json_file)
    return data
print('Starting command line, use "help . ." for help')
running = True
commandList = "add | sub | mult | div | exp | tetrate | python"
while running:
    try: 
        userInput = input(': ')
        tokens = userInput.split()
        command = tokens[0] 
        args = [(token) for token in tokens[1:]]
    except: print("unknown input error")
    try:
        arg1, arg2 = args
    except ValueError:
        print('Too many or too little args, 2 args required, if you want to not use an arg, use a "."')
    
    if command == "add":
        print(float(arg1) + float(arg2))
    
    if command == "sub":
        print(float(arg1) - float(arg2))
        
    if command == "mult":
        print(float(arg1) * float(arg2))        #math commands
        
    if command == "div":
        print(float(arg1) / float(arg2))
    
    if command == "exp":
        print(float(arg1) ** float(arg2))
        
    if command == "tetrate":
        for x in range(int(arg2)):
            arg1 = float(arg1) * float(arg1)
        print(arg1)
        
    if command == "python":
        exec(open(arg1).read())
        
    if command == "help":
        if arg1 == '.':
            if arg2 == '.':
                print('To see help about a command, type: "help [command] ." for list of commands type: "help command list"')
        
        if arg1 == 'command':
            if arg2 == 'list':
                print(commandList)
                
        if arg1 == 'add':
            print("Add: \n Description: Adds 2 numbers \n Syntax: add [num1] [num2]")
            
        if arg1 == 'sub':
            print("Sub: \nDescription: Subtracts 2 numbers \n Syntax : sub [num1] [num2]")
        
        if arg1 == 'mult':
            print("Mult: \nDescription: Multiplies 2 numbers \n Syntax : mult [num1] [num2]")
            
        if arg1 == 'div':
            print("Div: \nDescription: Divides 2 numbers \n Syntax : div [num1] [num2]")
        
        if arg1 == 'exp':
            print("Exp: \nDescription: Raises 1 number by another \n Syntax : exp [num1] [num2]")
            
        if arg1 == 'tetrate':
            print("Tetrate: \nDescription: Tetration \n Syntax : tetrate [num1] [num2]")
        
        if arg1 == 'python':
            print("Python: \nDescription: Runs a python script \n Syntax : python [path/to/program.py] .")
\$\endgroup\$
4
  • 2
    \$\begingroup\$ We need to know what the code is intended to achieve. To help reviewers give you better answers, please add sufficient context to your question, including a title that summarises the purpose of the code. We want to know why much more than how. The more you tell us about what your code is for, the easier it will be for reviewers to help you. The title needs an edit to simply state the task, rather than your concerns about the code. \$\endgroup\$ Commented Feb 9, 2021 at 20:26
  • \$\begingroup\$ (Know YAGNI?) \$\endgroup\$ Commented Feb 10, 2021 at 8:08
  • 1
    \$\begingroup\$ This is somewhat a case of "reinventing the wheel to be built in a square shape from smaller round wheels". A standard Python REPL is more capable, and already exists. \$\endgroup\$ Commented Feb 10, 2021 at 14:30
  • \$\begingroup\$ Is the enforcement of exactly 2 args for all commands (with suggestion to add dummy .) a feature you desire? Or just a just side effect of original implementation with all commands going through same path? \$\endgroup\$ Commented Feb 10, 2021 at 14:48

2 Answers 2

Reset to default
22
\$\begingroup\$

The first thing I notice with your code is that the code is a very long global script. To resolve this I'd start by moving your code into functions.

The actual calculations could be as simple as say:

def add(arg1, arg2):
    print(float(arg1) + float(arg2))

To then get the help information we could add a docstring.

def add(arg1, arg2):
    """
    Add:
     Description: Adds 2 numbers
     Syntax: add [num1] [num2]
    """
    print(float(arg1) + float(arg2))

We can get both the docstring and the output of the operation with Python. To get the docstring to be nicely formatted we can use textwrap.dedent.

>>> add("10", "5")
15.0
>>> import textwrap
>>> textwrap.dedent(add.__doc__.strip("\n"))
Add:
 Description: Adds 2 numbers
 Syntax: add [num1] [num2]

To then reduce the amount of lines of code we can bundle all these functions into a dictionary. And just index the dictionary to get a specific function.

COMMANDS = {
    "add": add,
    "sub": sub,
    # ...
}

fn = COMMANDS["add"]
fn("10", "5")

15.0

Whilst you can build the dictionary and execute the commands yourself, you could instead subclass cmd.Cmd. You will need to change the functions slightly to only take a string as input, and prefix do_ to any commands available to the commandline.

import cmd


class PyCMD(cmd.Cmd):
    intro = 'Welcome to pyCMD, user\nStarting command line, use "help" for help'
    prompt = ": "

    def do_add(self, arg):
        """
        Add:
         Description: Adds 2 numbers
         Syntax: add [num1] [num2]
        """
        arg1, arg2 = arg.split()
        print(float(arg1) + float(arg2))

PyCMD().cmdloop()

Welcome to pyCMD, user
Starting command line, use "help" for help
: help

Documented commands (type help <topic>):
========================================
add  help

: help add

        Add:
         Description: Adds 2 numbers
         Syntax: add [num1] [num2]
        
: add 10 5
15.0

The help is clearly a bit on the broken side. To fix this we can change the docstring after the function.

class PyCMD(cmd.Cmd):
    def do_add(self, arg):
        # ...

    do_add.__doc__ = textwrap.dedent(do_add.__doc__.rstrip(" ").strip("\n"))

This is a bit ugly and doing this for each function would be horrible. So we can create a function to do this.

def clean_doc(fn):
    fn.__doc__ = textwrap.dedent(fn.__doc__.rstrip(" ").strip("\n"))


class PyCMD(cmd.Cmd):
    def do_add(self, arg):
        # ...

    clean_doc(do_add)

This is still a bit on the ugly side so we can use @ to do this for us. This is called a decorator. This makes sense because we're decorating how __doc__ is seen. Note that we changed clean_doc to return fn.

def clean_doc(fn):
    fn.__doc__ = textwrap.dedent(fn.__doc__.rstrip(" ").strip("\n"))
    return fn


class PyCMD(cmd.Cmd):
    @clean_doc
    def do_add(self, arg):
        # ...

We can add another function like clean_doc, but this time make it a closure if you want to easily add your validation of two arguments.

import functools


def command(fn):
    @functools.wraps(fn)
    def wrapper(self, arg):
        args = [token for token in arg.split()]
        try:
            arg1, arg2 = args
        except ValueError:
            print('Too many or too little args, 2 args required, if you want to not use an arg, use a "."')
        else:
            return fn(self, arg1, arg2)
    return wrapper


class PyCMD(cmd.Cmd):
    @command
    @clean_doc
    def do_add(self, arg1, arg2):
        """
        Add:
         Description: Adds 2 numbers
         Syntax: add [num1] [num2]
        """
        print(float(arg1) + float(arg2))

import cmd
import textwrap
import functools


def command(fn):
    @functools.wraps(fn)
    def wrapper(self, arg):
        try:
            arg1, arg2 = arg.split()
        except ValueError:
            print('Too many or too little args, 2 args required, if you want to not use an arg, use a "."')
        else:
            return fn(self, arg1, arg2)
    return wrapper


def clean_doc(fn):
    fn.__doc__ = textwrap.dedent(fn.__doc__.rstrip(" ").strip("\n"))
    return fn


class PyCMD(cmd.Cmd):
    intro = 'Welcome to pyCMD, user\nStarting command line, use "help" for help'
    prompt = ": "

    @command
    @clean_doc
    def do_add(self, arg1, arg2):
        """
        Add:
         Description: Adds 2 numbers
         Syntax: add [num1] [num2]
        """
        print(float(arg1) + float(arg2))


PyCMD().cmdloop()

Welcome to pyCMD, user
Starting command line, use "help" for help
: help

Documented commands (type help <topic>):
========================================
add  help

: help add
Add:
 Description: Adds 2 numbers
 Syntax: add [num1] [num2]
: add 10 5
15.0

Other commands left for you to implement

\$\endgroup\$
2
  • 3
    \$\begingroup\$ What's great about this answer is separating the external behavior (prompt for a line, split, check number of words, support help command) from internal representation of possible commands. The internal representation is idiomatic python functions đź‘Ź. This is a widely applicable idea, and almost always makes both aspects clearer than when they're intermixed. \$\endgroup\$ Commented Feb 10, 2021 at 17:05
  • 1
    \$\begingroup\$ This is a very well written answer. I love how you've turned the code into a beautiful, extensible framework for adding future commands as simply as possible. I love the decorators too. I definitely learned a thing or two from this answer, even though I considered myself very fluent in python and pythonic design! \$\endgroup\$ Commented Feb 10, 2021 at 17:51
8
\$\begingroup\$

Review regarding the main question: reducing the ifs

Let me start by saying that Python does not support switch statements, which would have been very useful in this case. I would recommend reading this StackOverflow answer on how to get a similar result (I will be using that approach here too).

Something you should notice is that your if statements:

if command == "add":
    print(float(arg1) + float(arg2))

are running code that follows this pattern:

print(arg operand arg)

So it follows the question: how do we generalise that?

It turns out that we can use the operator to our advantage here:

import operator
def operation(a, b, operand):
   return operand(a, b)

Then you can map your "commands" to the different operands as such:

def get_operator(x):
   return {
     'add': operator.add,
     'multiply': operator.mul,
     ...
   }[x]

and finally you can reduce your multiple ifs with a single line of code:

operation(arg1, arg2, get_operator(command))

It must be noted that you could also have written:

get_operator(command)(arg1, arg2)

One can argue that one approach is better than the other for different reasons related to abstractions, decomposition, etc...

General Review

Argparse

Rather than doing all that tokenisation of the commands yourself, you should be using the argparse module. I am not going to explain here how to convert your code to use argparse, but if you follow the tutorial in the documentation it should be straighforward how to do that. That will also massively reduce the code you have written for the help menu.

Naming

In Python it is good convention to use snake_case for naming variables and functions. So the function SavetoDisk would become save_to_disk.

Lists and Enums

Your commandList is not a list. It is just a string separated by the | character. Rather consider using a true list and have enums rather than strings for your commands.

commands = [ Command.ADD, Command.MULTIPLY, ...]

Best of luck!

Update on Python Pattern Matching

I just found out that the pattern matching proposals (PEP 636 and companions PEP 634, PEP 635) and have been accepted for development just yesterday 08/02/2021!

\$\endgroup\$

You must log in to answer this question.