HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Calculate the intersection between two intervals on the same circle

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
circlesamethetwobetweenintervalscalculateintersection

Problem

I want to calculate the ratio of intersection between two intervals; the length of the intersection divided by the length of the shorter interval.

If two intervals do not intersect, the ratio is 0. If one interval is fully contained in the other, the ratio is 1, otherwise it is a number between 0 and 1.

These intervals are angular intervals that lie on the same circle.
So \$[-10°, 10°]\$ or \$[350°, 370°]\$ or \$[359990°, 360010°]\$ all represent the same interval.

Consequently, my method AngleIntervalIntersectionRatio() should return 1 when called with
\$[-10°, 10°]\$ and \$[350°, 370°]\$, since both intervals are contained in one another

Some other example values:

[-10°, 10°] and [0°, 360°]: return 1; (= intersection:20° / shortest interval:20°)
[-10°, 10°] and [0°, 180°]: return 0.5; (= intersection:10° / shortest interval:20°)
[-10°, 10°] and [90°, 180°]: return 0;
[-10°, 10°] and [350°, 360°]: return 1;
[-20°, 20°] and [10°, 350°]: return 0.5; (= intersection:20° / shortest interval:40°)
[-10°, 10°] and [-3600090°, -3600000°]: return 0.5;


I have two methods for this: IntervalIntersectionRatio() just calculates the intersection ratio of two intervals on an infinite line.
AngleIntervalIntersectionRatio() uses this function to calculate the intersection ratio of two intervals on a circle.

I am unhappy with my current code, mainly with two parts:

  • The code I use to normalize the angles to be between \$0°\$ and \$360°\$ uses some while loops. Anything I try with modulo just becomes very hard to understand at a glance.



  • Since the end of the interval can be above \$360°\$ after normalization, I have to call IntervalIntersectionRatio() 3 times to cover all possible cases of intersection. For example, with intervals such as \$[-20°, 20°]\$ and \$[10°, 350°]\$ (becomes \$[340°, 380°]\$ and \$[10°, 350°]\$ after normalizing, and the intersection is \$20°\$, not \$10°\$).



Is there anything I could to to make it more beautiful? I am not prima

Solution

Algorithm wise it looks quite good but I feel you could benefit from the single responsibility principle.

First thing that would get on my nerves would be passing the intervals in "non atomic form" e.g.:

double IntervalIntersectionRatio(double i1Start, double i1End, double i2Start, double i2End)


Instead I would want to have an interval class that has a start and an end member.

Now that you have got that class you could implement a "stupid" Interval intersection(Interval, Interval) function that works on plain intervals (without the circular logic).

Next thing to go about would be the normalization. That code duplication you have there is really a strong smell. Instead have a function to normalize a single number into interval [0, 360) and maybe one that does that for the start and end of an Interval returning the normalized Interval.

Now the only thing you must take care of is the case where one Interval wraps around the 360|0 border which would not be handled correctly by the "stupid" intersection function. So you would have another function that "unwraps" such an Interval by adding 360 to the end of it before passing the Interval to the intersection function.

Giving a length function for Intervals will make the code a bit more readable and most of your problems should be gone.

Disclaimer I did not think too deeply about this and there might be some edge cases that I missed here but the overall design improvement should make them easier to catch.

Code Snippets

double IntervalIntersectionRatio(double i1Start, double i1End, double i2Start, double i2End)

Context

StackExchange Code Review Q#54257, answer score: 5

Revisions (0)

No revisions yet.