CodeSOD: A Hardware Switch

Michele S. had recently worked on code for an engine-controlled device. Since the device had physically-moving parts operated by servos, the software had to be careful to not move anything out of bounds because it could actually break a physical piece of equipment.

Michele had written the low-level function which sets the position of a component.

 void PlcPart::set_target_position (uint16_t parPosition) { using boost::algorithm::clamp; const uint16_t min_pos = m_set_base_position; const uint16_t max_pos = min_pos + m_movement_range; vaeAssertRelease(parPosition >= min_pos and parPosition <= max_pos); m_target_position = clamp(parPosition, min_pos, max_pos); }

It validates that the requested position is in a suitable range, asserts if it's out-of-range, and then clamps it to the accepted range. It prevents physical damage to the hardware and safely notifies the programmer if they try to exceed the boundaries.

At some point, Michele discovered that the unit tests started having issues. Apparently, someone was annoyed at the assertion messages and took it upon himself to correct this. But, instead of fixing the root cause (calling set_target_position with bad values), he modified the function itself to look like this:

void PlcPart::set_target_position (uint16_t parPosition) { using boost::algorithm::clamp; const uint16_t min_pos = m_set_base_position; const uint16_t max_pos = min_pos + m_movement_range; if (parPosition > max_pos) parPosition = max_pos; vaeAssertRelease(parPosition >= min_pos and parPosition <= max_pos); m_target_position = clamp(parPosition, min_pos, max_pos);}

Assertions avoided! (Well, half of them anyway.)

[Advertisement] BuildMaster is more than just an automation tool: it brings together the people, process, and practices that allow teams to deliver software rapidly, reliably, and responsibly. And it's incredibly easy to get started; download now and use the built-in tutorials and wizards to get your builds and/or deploys automated!