0

I want to simplify the body of my current code which involves comparative operators. The operators were given to me in a string format, so I thought of using if/elif statements to return the result of the comparison.

Below is the body of my code:

if condition[1] == '<':
    return True if item[condition[0]] < condition[2] else False
elif condition[1] == '<=':
    return True if item[condition[0]] <= condition[2] else False
elif condition[1] == '==':
    return True if item[condition[0]] == condition[2] else False

...and the same format is also used for other available comparative operators such as != and >=.

I noticed that there is a repetition and this should be easy to solve, but I can't seem to think of a way to simplify this into a shorter code.

Tomerikoo
  • 18,379
  • 16
  • 47
  • 61
cauliflower
  • 41
  • 7
  • 1
    Does this answer your question? [Turn string into operator](https://stackoverflow.com/questions/1740726/turn-string-into-operator) – Tomerikoo Apr 27 '20 at 08:14
  • 1
    it did helped me understand the string-operator relation better, thanks for the comment :) – cauliflower Apr 27 '20 at 08:17

2 Answers2

2

Making use of the operator library together with renaming variables will make your code much more readable:

import operator

operations = {'<':  operator.lt,
              '<=': operator.le,
              '==': operator.eq}

key1, operation, operand2 = condition[:3]
operand1 = item[key1]

return operations[operation](operand1, operand2)

To handle unsupported operators, you can change the return statement to:

return operations.get(operation, lambda x,y: None)(operand1, operand2)

And you can change the lambda's return value to any default value you want.

Tomerikoo
  • 18,379
  • 16
  • 47
  • 61
  • Argh! You were faster than me ... I added to your answer the last thing remaining from mine: unpacking assignment. Feel free to revert if you disagree. – PiCTo Apr 27 '20 at 08:36
  • Note that, similarly to the other answer, you might want to use `.get()` instead of `[]` to mimic the behaviour of OP's code when the operator is not know by silently ignoring it (_i.e._ not returning or "falling of" thus returning `None`). – PiCTo Apr 27 '20 at 08:45
  • Thanks @PiCTo I modified your changes a bit to keep my original informative naming. The `get` is definitely a nice addition to add support for unhandled operators. – Tomerikoo Apr 27 '20 at 08:57
1

Your code is perfectly readable - there is no need to make it more complex.

However, if you wanted to, you can use a mapping to get rid off the if-statements:

cond_map = {
    '<': item[condition[0]].__lt__,
    '<=': item[condition[0]].__le__,
    '==': item[condition[0]].__eq__
}

And your return statement becomes:

return True if cond_map.get(condition[1])(condition[2]) else False

Or more simply:

return cond_map.get(condition[1])(condition[2])

Which is less code, but also less readable. I'd suggest sticking to the if-else ladder

rdas
  • 20,604
  • 6
  • 33
  • 46
  • _Which is less code, but also less readable._: readability comes in good part from good variable naming. Please refer to the other answer: is that really less readable than OP's code? In particular, it is undeniable that the code in both answers is more maintainable than the code from the question, which is an extremely import aspect of software development. – PiCTo Apr 27 '20 at 08:39
  • I'd take an if-else ladder over getting operators from a map to maintain anyday. But to each his own I guess :) – rdas Apr 27 '20 at 08:49
  • I agree that this is quite opinion-based. – PiCTo Apr 27 '20 at 09:58