HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Readability in creation of a long output string

Submitted by: @import:stackexchange-codereview··
0
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 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:

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.