The code below has poor clarity and violates the course coding style in several ways. Explanations are below the code and refer to the numbers ➊ embedded. Some minor improvements are demonstrated in the less-bad example; however, you should strive to make your code look like the good example.

Code


#include "fakerobot.h"

int main(void) {  
	while (1) {
		clear_display();
		display("Press OK");
		wait_for_button_down(OK_BUTTON);
		wait_for_button_up(OK_BUTTON);
		pause_ms(250);
		clear_display(); 
		display("Driving");
		zero_encoders();
		set_speed(BOTH_WHEELS, 20);
		while (read_encoder(LEFT_WHEEL) < 1050 && read_encoder(RIGHT_WHEEL) < 1050) 
			pause_ms(50); 
		set_speed(BOTH_WHEELS, 0);
		clear_display();
		display("Turning");
		set_speed(LEFT_WHEEL, 10);
		set_speed(RIGHT_WHEEL, -10);
		while (read_encoder(LEFT_WHEEL) < 22 && read_encoder(RIGHT_WHEEL) > -22)
			pause_ms(50);
		set_speed(BOTH_WHEELS, 0);
		clear_display(); 
		display("Driving"); 
		zero_encoders();
		set_speed(BOTH_WHEELS, 20);
		while (read_encoder(LEFT_WHEEL) < 2100 && read_encoder(RIGHT_WHEEL) < 2100)
			pause_ms(50);
		set_speed(BOTH_WHEELS, 0);
		clear_display();
		display("Turning");
		set_speed(LEFT_WHEEL, -10);
		set_speed(RIGHT_WHEEL, 10);
		while (read_encoder(LEFT_WHEEL) < -45 && read_encoder(RIGHT_WHEEL) > 45)
			pause_ms(50);
		set_speed(BOTH_WHEELS, 0);
		clear_display(); 
		display("Driving");
		zero_encoders();
		set_speed(BOTH_WHEELS, 20);
		while (read_encoder(LEFT_WHEEL) < 1050
				&& read_encoder(RIGHT_WHEEL) < 1050) 
			pause_ms(50);
	}
}

Clarity

➊ The code has no hierarchical structure. Everything is jammed into one big main method. Figuring out what the code does requires reading through every line, and everything is expressed at a very low level. There are no in-line comments to guide us.

The code is repetitive. Look at the sequences of statements starting at ➌, ➍, and ➎. In each case you’ll see six nearly-identical lines of code, which cause the robot display “Driving”, drive forward some distance and stop. If you look closely you will find two other blocks of statements related to turning. Worse yet, one of the driving blocks has an error: can you tell which one? Repeated code is a problem for many reasons:

  • each copy must be separately tested, since it’s separate code;
  • if we find an error in one copy, we need to propagate the fix to all the copies; and
  • there’s no way to re-use what we have written, other than by copy and paste (which gives us yet another copy to maintain).

Coding style

➋ There is no file header comment to explain the purpose of the file and indicate authorship and date.

The code includes single statement blocks like ➏ that are not enclosed in braces. While this is legal in C, it’s error-prone.

The code is riddled with “magic numbers”. What do the two 1050 at ➐ represent? Are they the same as the two 1050 at ➑? If we need to change one, do we change all four? Or just two? Or three? Are these 1050 related to the 2100, or is the fact that one is double the other just a coincidence?

Several lines exceed the maximum 80-character line length.