From dd5fc9432fcc59df5324e7a12c82226de5ad277c Mon Sep 17 00:00:00 2001 From: Colin Fletcher Date: Sun, 23 Nov 2014 19:16:42 +0000 Subject: [PATCH] Simplify Evoral::RangeList::subtract(), and make it pass amended tests The various conditionals in Evoral::RangeList::subtract() appear to have been there to work around (a) coverage() not always returning the correct value, & (b) the test suite assuming that the ->to point lies outside the range Now that these are both fixed, the implementation of subtract() becomes quite a bit clearer. I replaced the if()s with assert()s for now, but these shouldn't trip if coverage() is working as I expect. Also (attempt to) clarify the comments in subtract. --- libs/evoral/evoral/Range.hpp | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/libs/evoral/evoral/Range.hpp b/libs/evoral/evoral/Range.hpp index 0019604bb0..4cce6f661a 100644 --- a/libs/evoral/evoral/Range.hpp +++ b/libs/evoral/evoral/Range.hpp @@ -196,7 +196,7 @@ template struct /*LIBEVORAL_API*/ Range { Range (T f, T t) : from (f), to (t) {} T from; ///< start of the range - T to; ///< end of the range + T to; ///< end of the range (inclusive: to lies inside the range) }; template @@ -297,28 +297,33 @@ RangeList subtract (Range range, RangeList sub) switch (coverage (j->from, j->to, i->from, i->to)) { case OverlapNone: - /* The thing we're subtracting does not overlap this bit of the result, + /* The thing we're subtracting (*i) does not overlap this bit of the result (*j), so pass it through. */ new_result.add (*j); break; case OverlapInternal: - /* Internal overlap of the thing we're subtracting from this bit of the result, - so we might end up with two bits left over. + /* Internal overlap of the thing we're subtracting (*i) from this bit of the result, + so we should end up with two bits of (*j) left over, from the start of (*j) to + the start of (*i), and from the end of (*i) to the end of (*j). */ - if (j->from < (i->from - 1)) { - new_result.add (Range (j->from, i->from - 1)); - } - if (j->to != i->to) { - new_result.add (Range (i->to, j->to)); - } + assert (j->from < i->from); + assert (j->to > i->to); + new_result.add (Range (j->from, i->from - 1)); + new_result.add (Range (i->to + 1, j->to)); break; case OverlapStart: - /* The bit we're subtracting overlaps the start of the bit of the result */ - new_result.add (Range (i->to, j->to - 1)); + /* The bit we're subtracting (*i) overlaps the start of the bit of the result (*j), + * so we keep only the part of of (*j) from after the end of (*i) + */ + assert (i->to < j->to); + new_result.add (Range (i->to + 1, j->to)); break; case OverlapEnd: - /* The bit we're subtracting overlaps the end of the bit of the result */ + /* The bit we're subtracting (*i) overlaps the end of the bit of the result (*j), + * so we keep only the part of of (*j) from before the start of (*i) + */ + assert (j->from < i->from); new_result.add (Range (j->from, i->from - 1)); break; case OverlapExternal: