-2

I have a dataset I need to interpret in order to reconstruct a signal to analyse. To do this I use a large number of if, elif statements. I was wondering if there is a more elegant way to do this?

This is my code:

for line in data:
if line[:2] == '10':
    capstart = 1
if capstart == 1:
    if line[:2] == 'e3':
        reconstruct(int(line[2:6],16), int(line[6:10],16))
    elif line[:2] == '02':
        if line[8:] == '20':
            REDoffset = int(line[2:4],16)
            REDledlvl = int(line[4:6],16)
            REDgain = int(line[6:8],16)
        if line[8:] == '10':
            IRoffset = int(line[2:4],16)
            IRledlvl = int(line[4:6],16)
            IRgain = int(line[6:8],16)
    elif line[:2] == '10':
        if line[2:4] == '00':
            pulse_length_lo = int(line[4:],16)
        if line[2:4] == '01':
            pulse_length_hi = int(line[4:],16)
        if line[2:4] == '02':
            pulse_ir_on = int(line[4:],16)
        if line[2:4] == '03':
            pulse_red_on = int(line[4:],16)
        if line[2:4] == '04':
            pulse_led_switch_time = int(line[4:],16)
        if line[2:4] == '05':
            dark_worn_threshold = int(line[4:],16)
        if line[2:4] == '06':
            imu_accel_range = int(line[4:],16)
        if line[2:4] == '07':
            imu_gyro_range = int(line[4:],16)
        if line[2:4] == '08':
            ls_duration = int(line[4:],16)
        if line[2:4] == '09':
            imu_interval = int(line[4:],16)
        if line[2:4] == '0a':
            timestamp_lo = int(line[4:],16)
        if line[2:4] == '0b':
            timestamp_hi = int(line[4:],16)
        if line[2:4] == '0c':
            ADC_MAX = int(line[4:],16)
        if line[2:4] == '0d':
            ADC_offset = int(line[4:],16)
        if line[2:4] == '0e':
            lightsensor_has_red = int(line[4:],16)
        if line[2:4] == '0f':
            IR_worn_current = int(line[4:],16)
        if line[2:4] == '10':
            IR_worn_offset = int(line[4:],16)
        if line[2:4] == '11':
            IR_worn_threshold = int(line[4:],16)
        if line[2:4] == '12':
            accel_num_bits = int(line[4:],16)
        if line[2:4] == '13':
            gyro_num_bits = int(line[4:],16)
        if line[2:4] == 'ff':
            sensor_end_status = line[4:]
    else:
        other.append(line)

Note that the 'data' list contains a number of packets in hex, where the fisrt byte (first two chars in the string) indicate the packet type, and within each packet type, I need to separate the strings and isolate the data containded there.

I you could point me where to look it would be great! The code works, but I'd like it to be more elegant.

Danf
  • 1,409
  • 2
  • 21
  • 39
  • 2
    This looks like a case for [Code Review](https://codereview.stackexchange.com/). Your question would also benefit from a sample input and output, so people understand your data structure. – Mr. T Jan 08 '18 at 13:41
  • 1
    Note that code posted to Code Review **must** be a self-contained working program, complete with relevant sample data. – PM 2Ring Jan 08 '18 at 13:51
  • Yes, there _is_ a lot that can be done to improve that code, but to do it properly would require a little modification to the rest of your program. But for starters, you could store `line[2:4]` instead of re-calculating it 20 times. – PM 2Ring Jan 08 '18 at 13:57
  • You might also want to look into these threads: https://stackoverflow.com/questions/31748617/too-many-if-statements https://stackoverflow.com/questions/17166074/most-efficient-way-of-making-an-if-elif-elif-else-statement-when-the-else-is-don – Mr. T Jan 08 '18 at 14:00
  • I didn't provide sample data because I needed some code refinement olny, my code works, but is a mess! Thanks for the help! – Danf Jan 08 '18 at 17:23

2 Answers2

0

I'm deliberately glossing over details here. Looking just at the variables that depend on line[2:4]:

Start with a dictionary called line_2_4_lookup that maps the input numbers to variable names:

line_2_4_lookup = {
             '00':           "pulse_length_lo",
             '01':           "pulse_length_hi",
             '02':           "pulse_ir_on",
             '03':           "pulse_red_on",
             '04':           "pulse_led_switch_time",
             '05':           "dark_worn_threshold",
             '06':           "imu_accel_range",
             '07':           "imu_gyro_range",
             '08':           "ls_duration",
             '09':           "imu_interval",
             '0a':           "timestamp_lo",
             '0b':           "timestamp_hi",
             '0c':           "ADC_MAX",
             '0d':           "ADC_offset",
             '0e':           "lightsensor_has_red",
             '0f':           "IR_worn_current",
             '10':           "IR_worn_offset",
             '11':           "IR_worn_threshold",
             '12':           "accel_num_bits",
             '13':           "gyro_num_bits",
             'ff':           "sensor_end_status"}

Then instead of having variables like gyro_num_bits, store the incoming numbers in a dict called, say, input_values, so instead of gyro_num_bits your program will need to refer to input_values["gyro_num_bits"].

Then the decoding is just one line:

input_values[line_2_4_lookup[line[2:4]]] = int(line[4:],16)
BoarGules
  • 16,440
  • 2
  • 27
  • 44
0

Instead of using a zillion separate named variables for all those parameters we can store them in a dictionary, which I've named params. I store the parameter names in another dict named field_names, with the associated hex code as the key.

field_names = {
    '00': 'pulse_length_lo',
    '01': 'pulse_length_hi',
    '02': 'pulse_ir_on',
    '03': 'pulse_red_on',
    '04': 'pulse_led_switch_time',
    '05': 'dark_worn_threshold',
    '06': 'imu_accel_range',
    '07': 'imu_gyro_range',
    '08': 'ls_duration',
    '09': 'imu_interval',
    '0a': 'timestamp_lo',
    '0b': 'timestamp_hi',
    '0c': 'ADC_MAX',
    '0d': 'ADC_offset',
    '0e': 'lightsensor_has_red',
    '0f': 'IR_worn_current',
    '10': 'IR_worn_offset',
    '11': 'IR_worn_threshold',
    '12': 'accel_num_bits',
    '13': 'gyro_num_bits',
    'ff': 'sensor_end_status',
}

params = {}
capstart = False
for line in data:
    if line[:2] == '10':
        capstart = True
    if capstart:
        if line[:2] == 'e3':
            reconstruct(int(line[2:6], 16), int(line[6:10], 16))
        elif line[:2] == '02':
            if line[8:] == '20':
                params['REDoffset'] = int(line[2:4], 16)
                params['REDledlvl'] = int(line[4:6], 16)
                params['REDgain'] = int(line[6:8], 16)
            elif line[8:] == '10':
                params['IRoffset'] = int(line[2:4], 16)
                params['IRledlvl'] = int(line[4:6], 16)
                params['IRgain'] = int(line[6:8], 16)
        elif line[:2] == '10':
            code = line[2:4] 
            field = field_names[code]
            params[field] = line[4:] if field == 'sensor_end_status' else int(line[4:], 16) 
        else:
            other.append(line)

You haven't given us any sample data, so this code is untested, but this should get you started.

PM 2Ring
  • 54,345
  • 6
  • 82
  • 182
  • Great! I will try this then... Testing is not that important since all I wanted was to be pointed in the right direction to improve the code, the dictionary looks to be the way... I come from C, so my thinking is a bit different! – Danf Jan 08 '18 at 17:19
  • @Danf Well, I like to test the code I post, in case I made a dumb mistake. ;) I guess I could've created my own fake data, but that takes time. FWIW, in Python, it's quite common to use a dict where you'd use a switch in C. BTW, coming from C you may find this article helpful: [Facts and myths about Python names and values](http://nedbatchelder.com/text/names.html), which was written by SO veteran Ned Batchelder. It has a nice explanation of Python's data model, which has some important differences to the traditional data model that C uses. – PM 2Ring Jan 08 '18 at 17:30