"Python AutoClicker" Part 2

Question

I previously posted a question for codereview:

And I received some great feedback and changes to do in the code.

I have done many of the changes mentioned in the answers and here is the final code:

from pynput.keyboard import Controller
from time import sleep


def auto_click(key: str, n_clicks: int, delay: float):
    keyboard = Controller()

    for _ in range(n_clicks):
        keyboard.tap(key)
        sleep(delay)


def main():
    key = input("Key to be autopressed: ")

    try:
        n_clicks = int(input("Number of autopresses (integer): "))
    except ValueError:
        print("\n The number of autopresses should be an integer value, defaulting to 1. \n")
        n_clicks = 1

    try:
        delay = float(input("Delay between each autopress in seconds (integer/float): "))
    except ValueError:
        print("\n The delay between each autoclick should be an integer or a decimal value, defaulting to 1 (second). \n")
        delay = 1

    auto_click(key, n_clicks, delay)

if __name__ == '__main__':
    main()

What is it that you think should be made better in this code? Any feedback and suggestions would be much appreciated.

Here's the github repo:

https://github.com/SannanOfficial/AutoKeyPython

FMc · Accepted Answer · 2021-05-08 22:42:41Z

Your code is generally fine. I suspect you are not doing enough validation (for example, to forbid negative values or multi-character keys). Also, if you want to expand your toolkit, you can think about eliminating the duplication in your code by generalizing the input collection process.

Collecting user input involves a few common elements: an input prompt, a function to convert the string to a value, a function to validate that value, an error message in the event of bad input, and possibly some other stuff (max allowed attempts, fallback/default value in the event of bad input). If we focus on just the basics, we end up with a function along the lines of this sketch:

def get_input(convert, validate, prompt, errmsg):
    while True:
        try:
            x = convert(input(prompt + ': '))
            if validate(x):
                return x
            else:
                print(errmsg)
        except ValueError:
            print(errmsg)

With that in hand, all of the algorithmic detail will reside in that single function, and your main() will be little more than straightforward, step-by-step orchestration code.

def main():
    key = get_input(
        str.strip,
        lambda s: len(s) == 1,
        'Key to be autopressed',
        'Invalid, must be single character',
    )
    n_clicks = get_input(
        int,
        lambda n: n > 0,
        'Number of autopresses',
        'Invalid, must be positive integer',
    )
    delay = get_input(
        float,
        lambda n: n > 0,
        'Delay between autopresses',
        'Invalid, must be positive float',
    )
    auto_click(key, n_clicks, delay)

As shown in that sketch, I would also encourage you to favor a more compact style in messages to users. Brevity usually scales better across a variety of contexts than verbosity: it's easier to maintain; it's easier to make consistent across an entire application; and most people don't appreciate having to slog through long messages when an equally clear short one is possible.

Finally, you might also consider whether input() is the best mechanism to get this information from the user. In my experience, it is almost always preferable to take such values from the command-line, either from sys.argv directly or, more commonly, using a library like argparse. The advantages to this approach are numerous.

Thank you so much for the answer, mate. All of your suggestions make sense to me and I'll try to implement all of them If I can. — Syed M. Sannan - alt
– Syed M. Sannan - alt, Commented May 9, 2021 at 15:00

Omar Aflak · Accepted Answer · 2021-05-12 13:46:56Z

If n_clicks and delay have default values, you can put them directly in the signature of your auto_click function so that you can call it with the key parameter only. (I would also rename rename n_clicks to clicks, but that's a personal preference really:))

def auto_click(key: str, n_clicks: int = 1, delay: float = 1):
    keyboard = Controller()
    for _ in range(n_clicks):
        keyboard.tap(key)
        sleep(delay)

As @FMc mentionned, we don't usually use input() to take user input, but rather execute the program from the command line. There is an amazing library made by Google called fire (pip install fire) that generates the necessary code for you based on your function's signature:

import fire
from time import sleep
from pynput.keyboard import Controller


def auto_click(key: str, n_clicks: int = 1, delay: float = 1):
    keyboard = Controller()
    for _ in range(n_clicks):
        keyboard.tap(key)
        sleep(delay)


if __name__ == "__main__":
    fire.Fire(auto_click)

Now you can call your script from the command line like so:

python myscript.py --key A --n_clicks 5 --delay 1

Or, since you have default parameters:

python myscript.py --key A

Hope this helps :)

Thanks for the answer. I'll try to implement whatever I can that you suggested. — Syed M. Sannan - alt
– Syed M. Sannan - alt, Commented May 18, 2021 at 3:41

Stack Exchange Network

"Python AutoClicker" Part 2

2 Answers 2

You must log in to answer this question.

Linked

Hot Network Questions

"Python AutoClicker" Part 2

2 Answers 2

You must log in to answer this question.

Linked

Related

Hot Network Questions