Readability in creation of a long output string

Question

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:

1. 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 
   

2. 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 turns the {} on)\n'.format(
 level, device)
 Rpt += '\tDevice is measuring {}\n'.format(
 'in One Shot mode' if Conf & ONE_SHOT else 'continuously') 
 print Rpt 
 return Conf, TH, TL

My aim is for this to be as usable, readable and understandable as possible.

Answer 1

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'

• BeetDemGuise: I like your answer. However, since triple-quoted string is a literal string, the '\t's will print out. Also, why not just print tpl.format(sensor=sensor, ...)?
• janos: I tested it, it prints TABs for me. I agree with your other point, I updated my answer, thanks!
• BeetDemGuise: Intersting. I wasn't aware triple-quoted strings worked like that. +1 in any case.
• RolfBly: @janos Thanks! I think this is much more readable and accessible. Best answer.

Answer 2

I you wanted to take an OOP approach, you could create a class and then hide a lot of the repeated code in a single method:

class Bit():
 ''' Simple class that holds values associated with a given bit. '''
 def __init__(self, val, status_msg, true_val, false_val):
 self.val = val
 self.status_msg = status_msg
 # Make sure the values are stored in a list for easy formatting later.
 if hasattr(true_val, '__iter__'):
 self.true = true_val
 self.false = false_val
 else:
 self.true = [true_val]
 self.false = [false_val]
 def compare(self, config):
 # The star-notation takes an iterable and it says `take each of my elements
 # as INDIVIDUAL parameters`.
 return self.status_msg.format(*self.true if self.val & config else *self.false)

From here all you would need to do I call the the compare() method for each of your objects and format the print the returned information:

def read_config(bus, sensor, bits):
 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))
 # These would be declared wherever DONE, TH_BIT, etc. were declared.
 # Ideallythey would be passed into the function in a list. 
 nvb = Bit(NVB, '\tNon-volatile memory is {}\n', 'BUSY', 'not busy')
 done = Bit(DONE, '\tConversion is {}\n', 'done', 'in process')
 th_bit = Bit(TH_BIT, '\t{} measured {} degrees Celsius or more\n',
 ['HAVE', str(TH)], ['have NOT', str(TH)]))
 tl_bit = Bit(TL_BIT, '\t{} measured below {} degrees Celsius\n',
 ['HAVE', str(TL)], ['have NOT', str(TL)])
 pol_hi = Bit(POL_HI, '\tThermostat output is Active {} (1 turns the {} on)\n',
 ['HIGH', 'cooler'], ['LOW', 'heater'])
 one_shot = Bit(ONE_SHOT, '\tDevice is measuring {}\n', 'in One Shot mode', 'continuously')
 # I am assuming the objects above were passed into the funtion in
 # the `bits` list.
 print '\nStatus of DS1621 at address {}:'.format(sensor)
 for bit in bits:
 print bit.compare(config)
 return conf, TH, TL

As a general recommendation, I would pull as much repeated code into classes as possible. How you do this is up to you, but, based on your small snippet of code, this is how I would structure the classes.

• Jonas Schäfer: hasattr(true_val, "__iter__") is duck-typier than isinstance(true_val, list), and will work for exactly everything which works with the *-notation.
• RolfBly: @DarinDouglass I don't think it's good to hide mechanisms that help to understand the code. Also, hard-coding lists, specifically the )])) in str(TH)])), imho is not very pretty.
• BeetDemGuise: @RolfBly I agree with one of your points: hard-coding the list is definitely not the prettiest, however it was the solution that quickly came to mind when writing this answer. As for 'hiding mechanisms', I don't think that is what's happening here. What I'm doing is taking related info (data, statements, etc.) and bringing them into a single structure. This is the whole point behind OOP: provide a single point of access for evaluating and storing related info. Now, if your program is decently simple, I would recommend janos's answer. Otherwise, I would recommend my answer as it scales better.
• RolfBly: @DarinDouglass I accept your point and I very much appreciate your effort as well. But I often find OOP code unnecessarily abstract. Some time in the future, I'd be keen to debate that, but not here. Here, instead, maybe.