Best practice for "getting" functions
I am new to Python.
Suppose I have a dictionary that contains the administration state of a power supply. (OK = Enabled FAIL = Disabled).
There are several ways to write a "get" function:
1st way
is_power_supply_off(dictionary)
gets the admin state from dictionary.
returns true if turned off.
returns false if turned on.
is_power_supply_on(dictionary)
gets the admin state from dictionary.
returns true if turned on.
returns false if turned off.
Second way
is_power_supply_on_or_off(dictionary, on_or_off)
gets the admin state from dictionary.
returns true/false based on the received argument
Third way
get_power_supply_admin_state(dictionary)
gets the admin state from dictionary.
return value.
Then I can set in the function that calls the get function
if get_power_supply_admin_state() == turned_on/turned_off...
My questions:
Which of the above is considered best practice?
If all three are ok and it's just a matter of style, please let me know.
Is the first way "code duplication"? I am asking this because I can combine two functions as one (by adding an argument as in the second). However, IMO, the 1st way is more readable than the 2nd way.
I would be grateful if you can share your thoughts on each of the ways that I have indicated.
Thanks in advance!
source to share
I would say the best approach would be to only have a function is_power_supply_on
. Then, to check if it's off, you can do not is_power_supply_on(dictionary)
.
It could even be a lambda (assuming it state
is the admin status key) ::
is_power_supply_on = lambda mydict: mydict['state'].lower() == 'ok'
The problem with the first approach is that, as you say, it destroys codes.
The problem with the second approach is that at best you keep two characters compared to not
(if you use 0
or 1
for on_or_off
) and if you use a more idiomatic approach (like on=True
or on_or_off="off") you end up using more characters. Further, it results in slower and more complicated code since you need to do an
if`.
The problem with the third approach is that in most cases you will probably be wasting characters compared to just getting the value dict
with the key.
source to share
Even if this solution is not in your suggestions, I think the most pythonic way to create getters is to use properties. In this case, you will be able to know if the power is on or off, but the user will use this property since it was a simple member of the class:
@property
def state(self):
# Here, get whether the power supply is on or off
# and put it in value
return value
In addition, you can create two class constants PowerSupply.on = True
and PowerSupply.off = False
, which will simplify your code
source to share
The general style of Pythonic is not to repeat yourself unnecessarily, so certainly the first method seems pointless because it is actually confusing to follow (you should notice if it is included or not)
I would gravitate more towards
get_power_supply_admin_state(dictionary)
gets the admin state from dictionary
return value
And if I read this correctly, you can go even further.
power_supply_on(dictionary)
return the admin state from dictionary == turned on
This will evaluate to True if enabled and False otherwise, creating a trivial test, because then you can run
if power_supply_on(dictionary):
source to share
This is more Pythonic for storing a dictionary in a class:
class PowerSupply(object):
def __init__(self):
self.state = {'admin': 'FAIL'}
def turn_on(self):
self.state['admin'] = 'OK'
def is_on(self):
return self.state['admin'] == 'OK'
(add more methods as needed)
Then you can use it like this:
ps = PowerSupply()
if not ps.is_on():
# send an alert!
source to share
result = is_power_supply_off(state)
result = is_power_supply_on(state)
result = not is_power_supply_on(state) # alternatively, two functions are certainly not needed
I really prefer this name for readability. Let's just look at the alternatives, not in the function definition, but where the function is used.
result = is_power_supply_on_or_off(state, True)
pass
result = is_power_supply_on_or_off(state, False)
pass
if get_power_supply_admin_state(state):
pass
if not get_power_supply_admin_state(state):
pass
All of these codes require a map of what True
it False
means in this context. And to be honest, it's not so clear. In many embedded systems, 0
means true. What if this function parses the output from a system command? The value 0
(false) is an indicator of correct state / execution. As a result, intuitive True
means that "OK" does not always work. So my strong advice for the first option is the aptly named function.
Obviously you will have some kind of private function for example _get_power_supply_state_value()
. Both functions will call it and process it. But the point is - it will be hidden inside the module, which knows what it means, it is considering the power state. Implementation details and API users don't need to know.
source to share