A team was bringing forward an API for review, and one of the parts involved expressing a set of supported values in the form of three numbers:
- A minimum allowed value.
- An increment.
- A maximum allowed value.
The supported values are the minimum, integer multiples of the increment added to the minimum, up to and including the maximum.
For example, if the minimum is 5, the increment is 10, and the maximum is 30, then the valid values are
| 5 | Minimum |
| 15 | Minimum + 1 × Increment |
| 25 | Minimum + 2 × Increment |
| 30 | Maximum |
The team also noted that if the increment is zero, then the supported values are only the minimum and maximum.
I pointed out that this design tempts the user to divide by zero.
Here’s a function you might write:
int closestSupportedValue(int desired)
{
int nearest = minimum +
((desired - minimum + increment/2) / increment) * increment;
return std::clamp(nearest, minimum, maximum);
}
We first find the multiple of the increment beyond the minimum that is closest to our desired value. (You can adjust the rounding by changing the +increment/2 appropriately.) We then clamp that value to the allowable range.
This works great unless the increment is zero. And it’s possible that the developer never encounters a zero in all of their testing, say, because all of the hardware they tested their code on support a wide range of possible values. They never realized that they had to test on hardware that supported only 1 or 2 possible values.
I suggested that the team remove the zero from the API. If only two values are supported, then set the increment equal to maximum - minimum.¹ If only one value is supported, then set the increment to 1.
Bonus chatter: The power grid team fell into this trap. The PowerGridForecast class represents a series of PowerGridData objects, starting at the StartTime, where each prediction represents a length of time described by the BlockDuration. In other words, the nth element of the vector represents a prediction that starts at StartTime + n × BlockDuration and lasts for BlockDuration.
To write a function that finds the block that describes a specific point in time, you might come up with something like this:
PowerGridData FindForecastForDateTime(
PowerGridForecast forecast, DateTime time)
{
var elapsed = time - forecast.StartTime;
var index = elapsed / forecast.BlockDuration;
if (index < 0 || index >= forecast.Forecast.Count) {
return null; // no forecast for this index
}
return forecast.Forecast[index];
}
We determine the elapsed time since the start of the first forecast, determine which block index it belongs to, and return the element at that index, if it exists.
The team decided that if there was no forecast, then the Forecast property would return an empty vector (good), and the BlockDuration would be zero (bad).
The team figured that if there is no forecast, the block size is irrelevant since it’s describing something that doesn’t exist, so who cares if it’s zero? But we saw above that a zero block duration means that the calculation of the index performs a divide-by-zero.
Since the block size is irrelevant, they should have chosen a value that is unlikely to cause problems. Pick a nonzero block size that is typical of a valid block size, like say, one hour. In practice, most developers people are testing on machines with good Internet connectivity and which are physically located in areas that have good power grid forecast data. They don’t have test machines in a remote country.²
¹ Or really, you could set the increment to any positive value larger than maximum - minimum, though you don’t want to choose an excessively large value, because that risks integer overflow.
² And they are unlikely to have the budget to fly a developer to a faraway country just to ensure that this test case passes. “Boss, it’s important that we have a developer in Tahiti, to ensure we have proper test coverage.”
Footnote 2 reminds me of an experience I had when traveling to Indonesia which has a positive time zone offset. Our team of developers lives mostly in Eastern Time (UTC-5 standard or UTC-4 daylight). So we’ve never had problems with the following C# code:
<code>
This is valid in UTC or a negative offset of UTC because the corresponding UTC time is defined. For example, in Eastern Daylight Time, the result of that expression is “[0001/01/01 00:00:00 -05:00]” which in UTC is “[0001/01/01 05:00:00 +00:00]” which is after the earliest defined time in the current calendar. However, in Indonesia with...
oh this gives me nightmares and flashbacks about horribly designed scrollbars/slider, that have minimum/maximum properties and you always have to write a complicated algorithm when settings those by testing currently set values otherwise, "maximum must be larger than minimum", "value must be between min and max".... so you have to calculate which of the 3 to set first, second and last.
and on another note: I've seen developers "choose" to switch to floating point to avoid "division by zero" but then run into trouble and weird behavior many code lines later. Comparisons work fine with just one infinity, (to check like...
My first response to the spec before "For example" is that max need not be a candidate value, because (max - mini) need not be a multiple of step, e.g., mini = 0, max = 11, step = 5, then you can choose 0, 5, or 10. (The phrase "up to and including" means "up to and not excluding" in my book, [edited] i.e., I read it as { x | x = k * step + mini, k in N, x The team also noted that if the increment is zero, then the supported values are only the minimum...
The spec does discuss the case where (max – min) is not a multiple of step. In that case, the final step is a small one. In your example, the valid values would be 0, 5, 10, 11. And as luck would have it, the naive algorithm handles this correctly!
As I understand it, the original source of the data is a driver, and it’s unlikely that somebody would make a piece of hardware with “Oh we support 0, 5, 10, and 11.” (But who knows, weird things have happened.)