NSWI170 Computer Systems

Guidelines to write a better C/C++ code

previous curse | all curses | next curse

Unforgivable curse #3: Do not Repeat Yourselves (keep it DRY)

Many teachers have repeated to you that you should not repeat yourselves (at least not in the code). This may be a generally good practice not to copy the same thing over and over at least when it comes to matters of the digital world.

Motivation

When you repeat (possibly copy-paste) the same block of code, it makes it significantly harder to change later as you need to make the same modification over and over. It is also quite error-prone since it takes only a small omission and the duplicated code will get out of sync. Furthermore, you often need to make small adjustments in the copied code. That makes it very difficult to read and maintain as the eyes clearly see the same pattern but have a hard time spotting the tiny differences.

Explanation

When you have a block of code that needs to be executed multiple times, there are two things you should consider right away:

• Use a (for)loop to execute the block multiple times. Sometimes it might be difficult to spot, how to deal with minor differences in each iteration, but many situations can be solved by (constant) lookup tables (arrays).
• Use functions to wrap repeatedly used blocks. This goes hand in hand with functional decomposition. If you learned that lesson well, this will come as second nature to you.

On the other hand, there is an important red flag that could indicate that you are not doing the reduction of repeated code correctly. If you need to place branches (if-else statements) into the body of the loop or the function to distinguish individual loops/invocations, then something is terribly wrong and you should reconsider using a different approach.

Note that with the C/C++ skills you have gained in this course, it is not possible to eliminate all redundancies. For that, you would require OOP polymorphism, indirect function invocation (function pointers), or even templates. However, try to minimize the redundancies as much as possible (within a reason).

Examples

The following application handles three buttons that operate three individual LEDs. When a button is down, the corresponding LED is turned on, and vice versa. The application is wired so that Button #1 operates LED #3, Button #2 LED #2, and Button #3 LED #1. As the good code part demonstrates, the duplicities can be easily removed by loops.

int main() {
    initialize_pin(led_pin_1, OUTPUT);
    initialize_pin(led_pin_2, OUTPUT);
    initialize_pin(led_pin_3, OUTPUT);
    initialize_pin(button_pin_1, INPUT);
    initialize_pin(button_pin_2, INPUT);
    initialize_pin(button_pin_3, INPUT);

    if (get_pin_state(button_pin_1) == DOWN) {
        set_pin_state(led_pin_3, ON);
    } else {
        set_pin_state(led_pin_3, OFF);
    }    
    if (get_pin_state(button_pin_2) == DOWN) {
        set_pin_state(led_pin_2, ON);
    } else {
        set_pin_state(led_pin_2, OFF);
    }
    if (get_pin_state(button_pin_3) == DOWN) {
        set_pin_state(led_pin_1, ON);
    } else {
        set_pin_state(led_pin_1, OFF);
    }
}

constexpr int LED_PINS[] = { led_pin_1, led_pin_2, led_pin_3 };
constexpr LEDS = sizeof(LED_PINS) / sizeof(LED_PINS[0]);
constexpr int BUTTON_PINS[]
    = { button_pin_3, button_pin_2, button_pin_1 };
constexpr int BUTTONS
    = sizeof(BUTTON_PINS) / sizeof(BUTTON_PINS[0]);

int main() {
    for (int i = 0; i < LEDS; ++i) {
        initialize_pin(LED_PINS[i], OUTPUT);
    }
    for (int i = 0; i < BUTTONS; ++i) {
        initialize_pin(BUTTON_PINS[i], INPUT);
    }

    for (int i = 0; i < BUTTONS; ++i) {
        bool isDown = get_pin_state(BUTTON_PINS[i]) == DOWN;
        set_pin_state(LED_PINS[i], isDown ? ON : OFF);
    }
}

In the second example, each of the three buttons is doing something else. Therefore, using a loop that will hold a branch for each button is quite bad. A better solution is to place the common parts into a function and then simply call that function individually for each button. The main() function of the good code example could be considered also slightly redundant (the handle_button() is called three times in the same manner); however, without function pointers or polymorphism, that is the best we can do here.

int main() {
    // ...
    for (int i = 0; i < BUTTONS; ++i) {
        if (get_pin_state(BUTTON_PINS[i]) == DOWN) {
            if (!button_was_down[i]) {
                button_was_down[i] = true;
                if (i == 0) {
                    start_the_countdown();
                } else if (i == 1) {
                    stop_the_countdown();
                } else {
                    reset_counter();
                }
            }
        } else {
            button_was_down[i] = false;
        }
    }
}

bool handle_button(int buttonIndex) {
    if (get_pin_state(BUTTON_PINS[buttonIndex]) == DOWN) {
        if (!button_was_down[buttonIndex]) {
            button_was_down[buttonIndex] = true;
            return true;
        }
    } else {
        button_was_down[buttonIndex] = false;
    }
    return false;
}

int main() {
    // ...
    if (handle_button(0)) start_the_countdown();
    if (handle_button(1)) stop_the_countdown();
    if (handle_button(2)) reset_counter();
}