Mir

Code review comment for lp:~vanvugt/mir/Rectangle-contains-Rectangle

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

> What is special about empty rectangles?

> It seems perfectly reasonable that two rectangles of the same size and location
> each contain the other. (Or that a rectangle contains itself.)

It depends on what model we are using for rectangles in our discrete 2D domain. If we consider a rectangle to be a set (in the formal sense) of pixels, then we should be following the rules of sets. In this case, all rectangles with W=0 H=0 map to the empty set, so they should all contain each other.

If we are not following the set model, and then we can make our own rules for cases like this.

Anyway, since this is not likely to have any impact in practice (with the exception of "It just means that all code needs to be ready to handle empty rectangles, even if it's run conditionally after a rect.contains(rect) check"), I am OK with what is proposed, and we can reconsider if it becomes a problem.

I would like to see a comment describing the expected behavior, though (=> Needs Fixing).

review: Needs Fixing

« Back to merge proposal