-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: check near #194
fix: check near #194
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not crazy about the names in the public API: "FARM_ASSERT_NEAR" and "FARM_ASSERT_ABS_NEAR".
I think it's because I would use the "FARM_ASSERT_ABS_NEAR" whether or not I was dealing with numbers that were "near" each other. That's the use case that originally confused me about the API.
Here's another example:
double t_a = getTimestamp("a");
double t_b = getTimestamp("b");
// Sanity check that event a occurred within an hour of event b
FARM_ASSERT_ABS_NEAR(t0, t1, 60 * 60);
Could we follow the convention used in Catch2 and name them FARM_ASSERT_WITHIN_ABS
and FARM_ASSERT_WITHIN_REL
?
@isherman thanks for the naming suggestion, I adopted them as proposed. |
relativeCloseness(x, y) == relativeCloseness(y, x)
Fixes / clarifies issue discussed in this comment: #89 (comment)