Open Timmmm opened 5 years ago
Looking at this again, it looks like it is part of an attempt to make every empty interval equal to each other. For example the equality operator is:
template<class Type>
typename boost::enable_if<is_interval<Type>, bool>::type
operator == (const Type& left, const Type& right)
{
return (icl::is_empty(left) && icl::is_empty(right))
|| (lower_equal(left,right) && upper_equal(left,right));
}
A noble goal but I think this just leads to more WTFy behaviour than it prevents. For example:
void expand(boost::icl::interval<int>::type &ival) {
ival = {ival.lower(), ival.upper() + 1};
}
auto e0 = boost::icl::interval<int>::right_open(0, 0);
auto e4 = boost::icl::interval<int>::right_open(4, 4);
std::cout << (e0 == e4) << "\n"; // true - reasonable I guess, if a bit unexpected.
expand(e0);
expand(e4);
std::cout << (e0 == e4) << "\n"; // false - WTF??!
To avoid these issues it would be much cleaner if empty intervals were not special-cased.
From
interval.hpp
:This does not seem at all right. Consider this code and its results (the comments).
There are various WTFs in those results. Obviously the basic invariant
(a < b) == !(b < a)
is violated twice, and also the last 4 results are clearly inconsistent.Also, I think the basic idea that empty sets are treated specially is wrong, because it means you can't use them for things like
interval_set<>::lower_bound()
. I think the correct implementation would just be:But maybe there is code that relies on the current broken behaviour?