Skip to main content
Code Review

Return to Answer

added 3 characters in body
Source Link
BeetDemGuise
BeetDemGuise
  • 4.1k
  • 11
  • 29
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 isinstancehasattr(true_val, list'__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)
   

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 isinstance(true_val, list):
            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)
   

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)
added 38 characters in body
Source Link
BeetDemGuise
BeetDemGuise
  • 4.1k
  • 11
  • 29

YouI 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:

You could create a class and then hide a lot of the repeated code in a single method:

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:

Source Link
BeetDemGuise
BeetDemGuise
  • 4.1k
  • 11
  • 29

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 isinstance(true_val, list):
            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.