1
\$\begingroup\$

The program that contains the function I'm asking (handle_non_exact_answer) about has several instances of a class Test, with some options the user can set that determines the behavior of the class. If the class has certain options, we handle the class differently, so I deal with these cases in a separate function, which is below.

I would like to learn a) how to better write the function (ideally in the "most pythonic" way,) and b) how to resolve a pylint warning related to this function: Unnecessary "else" after "return".

import random
class Test:
    def __init__(self):
        self.precision = None
        self.expected_result = None


def handle_non_exact_answer(test, answer):
    if test.precision not in {"less", "more", "lesser", "greater"}:
        return False
    else:
        if test.precision == "less" and answer < test.expected_result:
            print("passed")
        elif test.precision == "more" and answer > test.expected_result:
            print("passed")
        elif test.precision == "lesser" and answer <= test.expected_result:
            print("passed")
        elif test.precision == "greater" and answer >= test.expected_result:
            print("passed")
        else:
            print("failed")
    return True

def main():
    test = Test()
    test.precision = "greater"
    test.expected_result = 0
    test1 = Test()
    test1.precision = "lesser"
    test1.expected_result = 0
    test2 = Test()
    test2.precision = "more"
    test2.expected_result = 0
    test3 = Test()
    test3.precision = "less"
    test3.expected_result = 0

    tests = [test, test1, test2, test3]

    for test in tests:
        handle_non_exact_answer(test, random.randint(-10,10))



if __name__ == "__main__":
    main()
\$\endgroup\$
0

1 Answer 1

5
\$\begingroup\$

That's a very unconventional usage of the term "precision". I think that "comparison" would be the appropriate word here.

The Test class is severely underdeveloped. The constructor fails to construct a useful object. Furthermore, representing the comparison as one of four magic strings is cryptic and unhelpful. On top of that, the Test class doesn't even know how to execute its own test, forcing you to write a separate handle_non_exact_answer() function.

It's rather weird that handle_non_exact_answer() returns True or False to indicate whether the comparison was one of the four recognized magic strings. I would consider an unrecognizable comparison as an error, and raise an exception — preferably when the Test is constructed, not when you try to perform the test.

Printing the "passed" or "failed" result limits the function's reusability. It should return True or False instead, and let the caller decide what to do with the result.

The if-else chain is more idiomatically done using a dictionary lookup.

Your main() test isn't very effective. You don't print the randomly chosen number, so it's hard to tell whether the results are correct. Also, you've used test to mean two different things: the first test ("greater than 0"), and also the iteration variable (for test in tests: …), which is unnecessarily confusing.

Suggested solution

import operator
import random

class ComparisonTest:
    COMPARATORS = {
        'less': operator.lt,
        'more': operator.gt,
        'lesser': operator.le,
        'greater': operator.ge,
    }

    def __init__(self, comparison, threshold):
        try:
            self._func = lambda n: self.COMPARATORS[comparison](n, threshold)
        except KeyError:
            raise ValueError('Invalid comparison: {0}'.format(comparison))

    def __call__(self, n):
        return self._func(n)

def main():
    n = random.randint(-1, 1)
    print('n = {0}'.format(n))
    test0 = ComparisonTest('greater', 0)
    test1 = ComparisonTest('lesser', 0)
    test2 = ComparisonTest('more', 0)
    test3 = ComparisonTest('less', 0)
    for test in [test0, test1, test2, test3]:
        print("passed" if test(n) else "failed")

if __name__ == "__main__":
    main()

More compact implementation

Here's a more compact way to write ComparisonTest, using functools.partial(). (Use the same main() function as above.)

from functools import partial
import operator

def ComparisonTest(comparison, threshold):
    try:
        return partial({
            'less': operator.gt,  # Comparisons are intentionally reversed
            'more': operator.lt,
            'lesser': operator.ge,
            'greater': operator.le,
        }[comparison], threshold)
    except KeyError:
        raise ValueError('Invalid comparison: {0}'.format(comparison))
\$\endgroup\$
1
  • \$\begingroup\$ Thank you for the help! That looks great. On the usage of "precision:" it originally started as a raw numerical precision (i.e. precision 0.2 to make sure that the value to be compared to is within .2 of the actual answer) but as the program evolved it became less accurate, as you stated. Thank you again! \$\endgroup\$
    – Luciano
    Oct 9, 2018 at 21:25

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.