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'

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.