Design function so variables get assigned values without large if/else block

Question

There must be a better way to do this:

def map_leds(self, i):
        if i[0] == 1:
            self.ledg_1_1.setVisible(True)
            self.ledr_1_1.setVisible(False)
        else:
            self.ledg_1_1.setVisible(False)
            self.ledr_1_1.setVisible(True)

###############################################

        if int(i[1]) == 1:
            self.ledg_1_2.setVisible(True)
            self.ledr_1_2.setVisible(False)
        else:
            self.ledg_1_2.setVisible(False)
            self.ledr_1_2.setVisible(True)

###############################################

        if int(i[2]) == 1:
            self.ledg_1_3.setVisible(True)
            self.ledr_1_3.setVisible(False)
        else:
            self.ledg_1_3.setVisible(False)
            self.ledr_1_3.setVisible(True)

###############################################

        if int(i[3]) == 1:
            self.ledg_2_1.setVisible(True)
            self.ledr_2_1.setVisible(False)
        else:
            self.ledg_2_1.setVisible(False)
            self.ledr_2_1.setVisible(True)

###############################################

        if int(i[4]) == 1:
            self.ledg_2_2.setVisible(True)
            self.ledr_2_2.setVisible(False)
        else:
            self.ledg_2_2.setVisible(False)
            self.ledr_2_2.setVisible(True)

###############################################

        if int(i[5]) == 1:
            self.ledg_2_3.setVisible(True)
            self.ledr_2_3.setVisible(False)
        else:
            self.ledg_2_3.setVisible(False)
            self.ledr_2_3.setVisible(True)

###############################################

        if int(i[6]) == 1:
            self.ledg_3_1.setVisible(True)
            self.ledr_3_1.setVisible(False)
        else:
            self.ledg_3_1.setVisible(False)
            self.ledr_3_1.setVisible(True)

###############################################

        if int(i[7]) == 1:
            self.ledg_3_2.setVisible(True)
            self.ledr_3_2.setVisible(False)
        else:
            self.ledg_3_2.setVisible(False)
            self.ledr_3_2.setVisible(True)

###############################################

if int(i[8]) == 1:
            self.ledg_3_3.setVisible(True)
            self.ledr_3_3.setVisible(False)
        else:
            self.ledg_3_3.setVisible(False)
            self.ledr_3_3.setVisible(True)

I have 9 LEDS, and I my function map_leds is fed an list of len 9 populated with 0s and 1s. If the first value is 1, the first LED should be green (this is a GUI, so an LED is actually a picture and I have to switch between both pictures, hence the "setVisible" function). If the first value is 0, then the LED (that should be displayed) is the red one. And so on for all the other values.

There must be a better way to actually do all this than have a huge if-else statement block, right?

Answer 1

The pattern I'm noticing is that the default is the red led on and green led off for all cases if i is not equal to 1. The leds are toggled if i equals 1.

LOOKUP_TABLE = {
    0: (1, 1),
    1: (1, 2),
    2: (1, 3),
    3: (2, 1),
    4: (2, 2),
    5: (2, 3),
    6: (3, 1),
    7: (3, 2),
    8: (3, 3),
}

LED_NAMING = "led{color}_{row}_{column}"

COLORS = {
    'green': 'g',
    'red': 'r'
}

def led_name(row=None, column=None, color=None, naming=LED_NAMING):
    """Get a led's name."""
    return naming.format(
        row=row,
        column=column,
        color=COLORS[color],
    )

def toggle(self, i, lookup=None):
    """Toggle the `i`th pair of leds."""

    lookup = lookup or LOOKUP_TABLE
    for led_index, led_switch in enumerate(i):
        row, column = lookup[led_index]

        green_led_name = led_name(row, column, 'green')
        red_led_name = led_name(row, column, 'red')

        green_led = getattr(self, green_led_name)
        red_led = getattr(self, red_led_name)

        if int(led_switch) == 1:
            green_led.setVisible(True)
            red_led.setVisible(False)
        else:
            green_led.setVisible(False)
            red_led.setVisible(True)

Here's a demonstration the code works:

class LED(object):
    def setVisible(self, value):
        self._visible = value

    @property
    def visible(self):
        return self._visible

class LEDMatrix(object):
    def __init__(self):
        for row, column in LOOKUP_TABLE.values():
            for color in COLORS:
                setattr(self, led_name(row, column, color), LED())

How you would use that:

>>> LEDMatrix.toggle = toggle # Inject our method in example class.
>>> ledmatrix = LEDMatrix()
>>> dir(ledmatrix) # We remove magic methods from display for clarity.
[
 'ledg_1_1',
 'ledg_1_2',
 'ledg_1_3',
 'ledg_2_1',
 'ledg_2_2',
 'ledg_2_3',
 'ledg_3_1',
 'ledg_3_2',
 'ledg_3_3',
 'ledr_1_1',
 'ledr_1_2',
 'ledr_1_3',
 'ledr_2_1',
 'ledr_2_2',
 'ledr_2_3',
 'ledr_3_1',
 'ledr_3_2',
 'ledr_3_3',
 'toggle'
]
>>> ledmatrix.ledg_1_1.visible # AttributeError
>>> ledmatrix.toggle([0, 1, 0, 1, 1, 1, 0, 1, 1])
>>> ledmatrix.ledg_1_1.visible # False
>>> ledmatrix.toggle([1, 1, 0, 1, 1, 1, 0, 1, 1])
>>> ledmatrix.ledg_1_1.visible # True

Answer 2

You want to do three things:

• to use the indices
• to transform a flat vector into a 3x3 array
• to transform int values to boleans

First of all, initialize your led_r as an array:

self.led_r = []
for i in range(3):
    self.led_r[i] = [your_obj_constructor() for j in range(3)]

Now you can use a two-liner:

for x in range(9):
    self.led_r[x // 3][x % 3].setVisible(i[x] == 1)

Answer 3

Maybe separate them into smaller functions?

def hand_ledg_1_1 (val):
    if val == 1:
        print("green")
        self.ledg_1_1.setVisible(True)
        self.ledr_1_1.setVisible(False)
    else:
        print("red")
        self.ledg_1_1.setVisible(False)
        self.ledr_1_1.setVisible(True)

def hand_ledg_1_2 (val):
    if int(val) == 1:
        self.ledg_1_2.setVisible(True)
        self.ledr_1_2.setVisible(False)
    else:
        self.ledg_1_2.setVisible(False)
        self.ledr_1_2.setVisible(True)

def hand_ledg_1_3 (val):
    if int(val) == 1:
        self.ledg_1_3.setVisible(True)
        self.ledr_1_3.setVisible(False)
    else:
        self.ledg_1_3.setVisible(False)
        self.ledr_1_3.setVisible(True)

def hand_ledg_2_1 (val):
    if int(val) == 1:
        self.ledg_2_1.setVisible(True)
        self.ledr_2_1.setVisible(False)
    else:
        self.ledg_2_1.setVisible(False)
        self.ledr_2_1.setVisible(True)

def hand_ledg_2_2 (val):
    if int(val) == 1:
        self.ledg_2_2.setVisible(True)
        self.ledr_2_2.setVisible(False)
    else:
        self.ledg_2_2.setVisible(False)
        self.ledr_2_2.setVisible(True)

def hand_ledg_2_3 (val):
    if int(val) == 1:
        self.ledg_2_3.setVisible(True)
        self.ledr_2_3.setVisible(False)
    else:
        self.ledg_2_3.setVisible(False)
        self.ledr_2_3.setVisible(True)

def hand_ledg_3_1 (val):
    if int(val) == 1:
        self.ledg_3_1.setVisible(True)
        self.ledr_3_1.setVisible(False)
    else:
        self.ledg_3_1.setVisible(False)
        self.ledr_3_1.setVisible(True)

def hand_ledg_3_2 (val):
    if int(val) == 1:
        self.ledg_3_2.setVisible(True)
        self.ledr_3_2.setVisible(False)
    else:
        self.ledg_3_2.setVisible(False)
        self.ledr_3_2.setVisible(True)

def hand_ledg_3_3 (val):
    if int(i[8]) == 1:
        print("green1")
        self.ledg_3_3.setVisible(True)
        self.ledr_3_3.setVisible(False)
    else:
        print("red1")
        self.ledg_3_3.setVisible(False)
        self.ledr_3_3.setVisible(True)

def map_leds(self, i):
    hand_ledg_1_1(i[0])
    hand_ledg_1_2(i[1])
    hand_ledg_1_3(i[2])
    hand_ledg_2_1(i[3])
    hand_ledg_2_2(i[4])
    hand_ledg_2_3(i[5])
    hand_ledg_3_1(i[6])
    hand_ledg_3_2(i[7])
    hand_ledg_3_3(i[8])

It's easier to read at least