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 allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!