patternpythonMinor
Readability in creation of a long output string
Viewed 0 times
readabilitylongoutputcreationstring
Problem
Below is a function from a driver that I wrote for and I2C temperature sensor.
The function takes as input the name of the bus and the device's bus address, reads it's status register, and then reports the status verbosely. (It also returns a values of interest, but that's not important now).
I have two questions about this:
-
I'm using lots of
or, worse (as in 'much more lines and no better readability'):
-
Same for using this way of constructing the output string. I see people get upset when you do this:
But in this case, it's concise, I think it's readable, it works. I don't see anything wrong with it. Is there?
Here's the full function.
```
def read_config(bus, sensor):
Conf = bus.read_byte_data(sensor, ACCESS_CONFIG)
TH = decode_DS(bus.read_word_data(sensor, ACCESS_TH))
TL = decode_DS(bus.read_word_data(sensor, ACCESS_TL))
Rpt = '\nStatus of DS1621 at address {}:\n'.format(sensor)
Rpt += '\tConversion is {}\n'.format(
'done' if Conf & DONE else 'in process')
Rpt += '\t{} measured {} degrees Celsius or more\n'.format(
'HAVE' if Conf & TH_BIT else 'have NOT', str(TH))
Rpt += '\t{} measured below {} degrees Celsius\n'.format(
'HAVE' if Conf & TL_BIT else 'have NOT', str(TL))
Rpt += '\tNon-volatile memory is {}\n'.format(
'BUSY' if Conf & NVB else 'not busy')
if Conf & POL_HI:
level, device = 'HIGH', 'cooler'
else:
level, device = 'LOW', 'heater'
Rpt += '\tThermostat output is Active {} (1 t
The function takes as input the name of the bus and the device's bus address, reads it's status register, and then reports the status verbosely. (It also returns a values of interest, but that's not important now).
I have two questions about this:
-
I'm using lots of
comment text {}'.format('this' if Condition else 'that'). I gather this is frowned upon, but in this case I think readability is optimal. The alternative would be statusthing = 'this' if Condition else 'that'
'comment text {}'.format(statusthing)or, worse (as in 'much more lines and no better readability'):
if Condition:
statusthing = 'this'
else:
statusthing = 'that'
Rpt += 'comment text %s' % statusthing-
Same for using this way of constructing the output string. I see people get upset when you do this:
Rpt = 'eggs, bacon, sausage \n'
.
.
Rpt += 'sausage'But in this case, it's concise, I think it's readable, it works. I don't see anything wrong with it. Is there?
Here's the full function.
```
def read_config(bus, sensor):
Conf = bus.read_byte_data(sensor, ACCESS_CONFIG)
TH = decode_DS(bus.read_word_data(sensor, ACCESS_TH))
TL = decode_DS(bus.read_word_data(sensor, ACCESS_TL))
Rpt = '\nStatus of DS1621 at address {}:\n'.format(sensor)
Rpt += '\tConversion is {}\n'.format(
'done' if Conf & DONE else 'in process')
Rpt += '\t{} measured {} degrees Celsius or more\n'.format(
'HAVE' if Conf & TH_BIT else 'have NOT', str(TH))
Rpt += '\t{} measured below {} degrees Celsius\n'.format(
'HAVE' if Conf & TL_BIT else 'have NOT', str(TL))
Rpt += '\tNon-volatile memory is {}\n'.format(
'BUSY' if Conf & NVB else 'not busy')
if Conf & POL_HI:
level, device = 'HIGH', 'cooler'
else:
level, device = 'LOW', 'heater'
Rpt += '\tThermostat output is Active {} (1 t
Solution
How about something like this:
This is PEP8 compliant, and I think more readable.
Also, I think there's nothing wrong with this kind of use:
def read_config(bus, sensor):
tpl = '''Status of DS1621 at address {sensor}:
\tConversion is {done}
\t{have_th} measured {th} degrees Celsius or more
\t{have_tl} measured below {tl} degrees Celsius
\tNon-volatile memory is {busy}'''
print tpl.format(sensor=sensor,
done='done' if Conf & DONE else 'in process',
have_th='HAVE' if Conf & TH_BIT else 'have NOT',
th=TH,
have_tl='HAVE' if Conf & TL_BIT else 'have NOT',
tl=TL,
busy='BUSY' if Conf & NVB else 'not busy',
)This is PEP8 compliant, and I think more readable.
Also, I think there's nothing wrong with this kind of use:
'something' if condition else 'otherthing'Code Snippets
def read_config(bus, sensor):
tpl = '''Status of DS1621 at address {sensor}:
\tConversion is {done}
\t{have_th} measured {th} degrees Celsius or more
\t{have_tl} measured below {tl} degrees Celsius
\tNon-volatile memory is {busy}'''
print tpl.format(sensor=sensor,
done='done' if Conf & DONE else 'in process',
have_th='HAVE' if Conf & TH_BIT else 'have NOT',
th=TH,
have_tl='HAVE' if Conf & TL_BIT else 'have NOT',
tl=TL,
busy='BUSY' if Conf & NVB else 'not busy',
)'something' if condition else 'otherthing'Context
StackExchange Code Review Q#49840, answer score: 3
Revisions (0)
No revisions yet.