5
\$\begingroup\$

What do you think about this code? Do you have some advices and remarks for me about them? I have started learning python recently.

types = {
    "str": str,
    "int": int,
    "float": float,
    "complex": complex
}


def my_input(kind, msg, msg_wrong, detail):
    """ Add-in for input(). Performs validation of the data entered.
    :param kind: data type. The keys of the dictionary types. 
                 If an error occurs, a str is expected.
    :param msg: a welcome message, such as "Enter a number -> "
    :param msg_wrong: own message in case of error
    :param detail: detailed error description (True or False)
    :return: entered value with type kind
    """
    method = types.get(kind, str)
    while True:
        try:
            value = method(input(msg))
            break
        except ValueError as e:
            print(msg_wrong, detail * str(e))
    return value


msg = "-> "
msg_wrong = "Error"

print(my_input("int", msg, msg_wrong, False))
print(my_input("hello", msg, msg_wrong, True))  # wrong type, will str
print(my_input("complex", msg, msg_wrong, True))

"""
-> hi
Error 
-> 15
15
-> some text
some text
-> some text 2
Error complex() arg is a malformed string
-> 4-j
(4-1j)
"""
\$\endgroup\$
3
\$\begingroup\$
  1. Your function looks usable, useful and follows PEP8, well done.
  2. Coupling types and kind together seems like a poor descision. Since functions are firstclass in Python you can just pass the function.

    This would change your functions to something like:

    my_input(int, msg, msg_wrong, False)
    

    This would have the drawback that if you don't pass a valid function, then it will blow-up. But has the added benifit of allowing more types with less code.

    import datetime
    my_input(datetime.fromisoformat, msg, msg_wrong, False)
    

    You can also fix the drawback by changing kind to an argument with a default value.

    def my_input(msg, msg_wrong, detail, kind=str)
    my_input(msg, msg_wrong, True)
    
  3. It's not immediatly clear why you would want to log only certain errors with the detail argument.

    If I were to show or silence errors I would use the logging module. I'll leave configuring the logger to you, but usage would be:

    import logging
    
    logger = logging.getLogger(__name__)
    logger.setLevel(logging.INFO)
    
    ...
    
    def my_input(kind, msg, msg_wrong):
        method = types.get(kind, str)
        while True:
            try:
                value = method(input(msg))
                break
            except ValueError as e:
                logger.debug(e)
                print(msg_wrong)
        return value
    
    ...
    
    print(my_input("int", msg, msg_wrong))
    logger.setLevel(logging.DEBUG)
    print(my_input("hello", msg, msg_wrong))
    print(my_input("complex", msg, msg_wrong))
    

    It's not apparent to me why you'd like to log some but not other errors. If being able to change the level of different calls to my_input is of the upmost importance then it'd make more sense to me for you to pass the logging level.

    def my_input(kind, msg, msg_wrong, level=logging.DEBUG):
        ...
                logger.log(level, e)
    
  4. I'm not used to seeing assignment, break and then a return out of a while true loop. When I do this I only use return.

import logging
logger = logging.getLogger(__name__)
logger.setLevel(logging.INFO)


def my_input(msg, msg_wrong, cast=str):
    while True:
        try:
            return cast(input(msg))
        except ValueError as e:
            logger.debug(e)
            print(msg_wrong)


msg = "-> "
msg_wrong = "Error"

print(my_input(msg, msg_wrong, int))
print(my_input(msg, msg_wrong))
print(my_input(msg, msg_wrong, complex))
\$\endgroup\$
  • \$\begingroup\$ What do you mean by word "cast"? I don't understand it with translation to my language. If I do how your fourth example, IDE PyCharm gives me notice "Expected type Type[str] got Type[int] instead". I add in docstring ":type cast: str, int, float, complex" (for Python 3) and notice disappears. Is it a right decision? \$\endgroup\$ – Owl May 26 at 7:51
  • \$\begingroup\$ Hmm.. If I write ":type cast: str, " that notice disappears too. \$\endgroup\$ – Owl May 26 at 8:08
  • \$\begingroup\$ @Owl Here's a Wikipedia page for more information, I'm unsure if it has been translated into your language however. In short a = 1; b = str(a) "casts" (converts) a from int to string and assigns it to b. I think using cast: Type = str would also fix the problem. \$\endgroup\$ – Peilonrayz May 26 at 13:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.