2
\$\begingroup\$

I wrote a little module for making quick CLIs in Python and I wanted to get your thoughts on areas of improvement (as well as to see what can break it).

One of the issues I had from the start was with looping and multi-threading, I wound up just adding a 1 second delay to make sure I've enough time to do some basic processing, but it seems a bit hackish. I'd like to perform some basic tests on the variable first (e.g. make sure it's not going to cause an overflow) and then actually copy the variable into an entirely separate memory space, although I'm not terribly knowledgeable about python's memory management practices and could use some guidance in the right direction.

A quick brief on how it works, functions which are to be made available to the user are stored in a dictionary with command:function key-pair, if a user enters a command which isn't recognized it will simply print the help screen message, right now it only has support for space separated values. It should be fairly trivial to add additional deliminator functionality like commas, dashes, etc. but I am also a bit worried about adding too many features as well and making it cumbersome to work with.

You can download the source on the GitHub repo or install with pip/pip3:

$ pip3 install -i https://test.pypi.org/simple/ pycli

And I've included the source code for reference:

#!/usr/bin/python3
"""Basic CLI in Python3"""

import threading as th
import time

cont=True
text=None

class InputThread:
    def __init__(self):
        global cont
        global text
        while True:
            self.loop_thread()
            time.sleep(1)
            text=None
            cont=True

    def loop_thread(self):
        global cont
        global text
        text = input()
        cont=False


class CLI:
    def __init__(self):
        self.cmds = {
            'help':self.help,
            }
        self.init_message = ''

    def help(self, args=None):
        """Use help 'command' for more information on a specific command."""
        if args==None:
            print("Available Commands:")
            for key in self.cmds.keys():
                print(key)
            print(self.help.__doc__)

        else:
            print(self.cmds[args[0]].__doc__)

    def set_message(self, message):
        """Sets the initialization message"""
        self.init_message = message

    def add_function(self, name, function):
        """Adds a function to the command dictionary"""
        self.cmds[name] = function

    def cli(self, a):
        global cont
        args = a.split(' ')
        if args[0] not in self.cmds.keys():
            print('Command "', args[0], '" Not Found.')
            self.help()
        else:
            if len(args) == 1:
                self.cmds[args[0]]()
            else:
                self.cmds[args[0]](args[1:])
        cont=True

    def loop(self):
        global cont
        global text
        print(self.init_message)
        th.Thread(target=InputThread, args=(), name='user_input_thread', daemon=True).start()
        while True:
            if not cont:
                self.cli(text)

if __name__ in '__main__':
    x = CLI().loop()

and here's an example CLI made using the module:

#!/usr/bin/python3

from pycli import CLI as cli
import random

def random_function(x, y):
    """Generates a random number between x and y.
    Usage:
      random x y
    """
    print(random.randint(x,y))

if __name__ in '__main__':
    x = cli()
    x.set_message("""Welcome to my super awesome python CLI!
With this new tool you can quickly generate command line
interfaces that capture user input without having to figure
out how to handle all those inputs yourself!
""")
    x.add_function('random', random_function)
    x.loop()

I've only tested this on Linux Mint (don't have Windows or Mac), so if you've got one of those OS's can you let me know if the code works as expected and any bugs?

\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

Your script consumes a lot of CPU due to the while loops. Using just the input function in a loop should be good enough for your purpose. I can't see the need for threading in a script that is quite basic for the moment.

These:

global cont
global text

don't belong in your class. A class is normally supposed to be self-contained and shouldn't have to rely on global variables defined elsewhere. They should go inside the class body. On the other hand you can pass parameters to your class instantiation method or to some class functions.

By the way text is not a good choice of variable since many objects have a text property and be careful with reserved keywords.


Exception handling is lacking. Any quality script should have at least basic exception handling (ten lines of code could do).


Validation of user input is also lacking. If I type anything after help I have a KeyError exception and your script crashes too easily. The dictionary selection is rather clumsy but see below for suggestions.


Structure: I am wondering why you split the code in two classes ? InputThread could have been embedded in CLI as a subclass or simply implemented as a function. But they are not independent from each other.


Style: after:

if __name__ in '__main__':

the norm is to call main(), so it is customary to have a main method.

You don't call a class like that:

x = CLI().loop()

Instead:

x = CLI()

with optional parameters inside the parentheses, or you run the code in multiple steps eg:

# instantiation
x = CLI()
# or:
x = CLI(prompt="Prompt message goes here")
x.start()
x.stop()

Because you can perfectly instantiate a class without immediately running it.


Naming conventions: variable names (a, text) are not always well-chosen and meaningful.


Misc: your script does not recognize commands in uppercase. I would always convert the commands to lowercase and use strip to trim them (here the split command takes care of that).

Overall impression: I think that this is a way of reinventing the wheel. There are modules that already exist for interactive cli programs, for example PyInquirer. There is also clint and quite a few others (I have not tested and compared all of them).
So I think I would build on the more mature solutions available, unless the problem you are trying to solve cannot be addressed adequately by the solutions that already exist.

An alternative would be to use feature-rich argparse. Because it can be used not only for command line options but in functions too (I haven't done that yet though but it's worth investigating). To give you an idea have a look at this post on SO: How to pass parameters to python function using argparse?. Also have a look at the Sub-commands section for more elaborate scenarios. As you can see this module is very powerful and very often underutilized.

It seems to me that it could have fulfilled all your desired functionality, with increased flexibility since the commands could be passed in a different order, as long as the grouping of dependent parameters is respected.

\$\endgroup\$
1
  • \$\begingroup\$ One of the reasons I'm using threading is to allow me to run (somewhat) asynchronous code to build a hardware abstraction layer, rather than stopping execution to get user input we're simply checking to see if the "cont"inue flag is pulled low. My idea is to basically implement something like the Serial.available() function on arduino in Python \$\endgroup\$
    – Noah Wood
    Commented Apr 27, 2020 at 5:52
1
\$\begingroup\$

Python originally works on windows just like it would on linux, the only differences being system-specific libraries and python extensions.

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